• Jakub Kicinski's avatar
    gianfar: remove faulty filer optimizer · 1f2b7293
    Jakub Kicinski authored
    Current filer rule optimization is broken in several ways:
     (1) Can perform reads/writes beyond end of allocated tables.
         (gianfar_ethtool.c:1326).
    
    (2) It breaks badly for rules with more than 2 specifiers
         (e.g. matching ip, port, tos).
    
    Example:
    # ethtool -N eth2 flow-type udp4 dst-ip 10.0.0.1 dst-port 1 tos 1 action 1
    Added rule with ID 254
    # ethtool -N eth2 flow-type udp4 dst-ip 10.0.0.2 dst-port 2 tos 2 action 9
    Added rule with ID 253
    # ethtool -N eth2 flow-type udp4 dst-ip 10.0.0.3 dst-port 3 tos 3 action 17
    Added rule with ID 252
    # ./filer_decode /sys/kernel/debug/gfar1/filer_raw
    00: MASK == 00000210 AND         Q:00           ctrl:00000080 prop:00000210
    01: FPR  == 00000210 AND CLE     Q:00           ctrl:00000281 prop:00000210
    02: MASK == ffffffff AND         Q:00           ctrl:00000080 prop:ffffffff
    03: DPT  == 00000003 AND         Q:00           ctrl:0000008e prop:00000003
    04: TOS  == 00000003 AND         Q:00           ctrl:0000008a prop:00000003
    05: DIA  == 0a000003 AND         Q:11           ctrl:0000448c prop:0a000003
    06: DPT  == 00000002 AND         Q:00           ctrl:0000008e prop:00000002
    07: TOS  == 00000002 AND         Q:00           ctrl:0000008a prop:00000002
    08: DIA  == 0a000002 AND         Q:09           ctrl:0000248c prop:0a000002
    09: DIA  == 0a000001 AND         Q:00           ctrl:0000008c prop:0a000001
    0a: DPT  == 00000001 AND         Q:00           ctrl:0000008e prop:00000001
    0b: TOS  == 00000001     CLE     Q:01           ctrl:0000060a prop:00000001
    ff: MASK >= 00000000             Q:00           ctrl:00000020 prop:00000000
    
    (Entire cluster gets AND-ed together).
    
     (3) We observed that the masking rules it generates do not
         play well with clustering on P2020.  Only first rule
         of the cluster would ever fire.  Given that optimizer
         relies heavily on masking this is very hard to fix.
    
    Example:
    # ethtool -N eth2 flow-type udp4 dst-ip 10.0.0.1 dst-port 1  action 1
    Added rule with ID 254
    # ethtool -N eth2 flow-type udp4 dst-ip 10.0.0.2 dst-port 2  action 9
    Added rule with ID 253
    # ethtool -N eth2 flow-type udp4 dst-ip 10.0.0.3 dst-port 3  action 17
    Added rule with ID 252
    # ./filer_decode /sys/kernel/debug/gfar1/filer_raw
    00: MASK == 00000210 AND         Q:00           ctrl:00000080 prop:00000210
    01: FPR  == 00000210 AND CLE     Q:00           ctrl:00000281 prop:00000210
    02: MASK == ffffffff AND         Q:00           ctrl:00000080 prop:ffffffff
    03: DPT  == 00000003 AND         Q:00           ctrl:0000008e prop:00000003
    04: DIA  == 0a000003             Q:11           ctrl:0000440c prop:0a000003
    05: DPT  == 00000002 AND         Q:00           ctrl:0000008e prop:00000002
    06: DIA  == 0a000002             Q:09           ctrl:0000240c prop:0a000002
    07: DIA  == 0a000001 AND         Q:00           ctrl:0000008c prop:0a000001
    08: DPT  == 00000001     CLE     Q:01           ctrl:0000060e prop:00000001
    ff: MASK >= 00000000             Q:00           ctrl:00000020 prop:00000000
    
    Which looks correct according to the spec but only the first
    (eth id 252)/last added rule for 10.0.0.3 will ever trigger.
    As if filer did not treat the AND CLE as cluster start but
    also kept AND-ing the rules.  We found no errata covering this.
    
    The fact that nobody noticed (2) or (3) makes me think
    that this feature is not very widely used and we should just
    remove it.
    Reported-by: default avatarAleksander Dutkowski <adutkowski@gmail.com>
    Signed-off-by: default avatarJakub Kicinski <kubakici@wp.pl>
    Acked-by: default avatarClaudiu Manoil <claudiu.manoil@freescale.com>
    Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
    1f2b7293
gianfar_ethtool.c 40.8 KB