• Boris Sukholitko's avatar
    net/sched: cls_flower: Remove match on n_proto · 0dca2c74
    Boris Sukholitko authored
    The following flower filters fail to match packets:
    
    tc filter add dev eth0 ingress protocol 0x8864 flower \
    	action simple sdata hi64
    tc filter add dev eth0 ingress protocol 802.1q flower \
    	vlan_ethtype 0x8864 action simple sdata "hi vlan"
    
    The protocol 0x8864 (ETH_P_PPP_SES) is a tunnel protocol. As such, it is
    being dissected by __skb_flow_dissect and it's internal protocol is
    being set as key->basic.n_proto. IOW, the existence of ETH_P_PPP_SES
    tunnel is transparent to the callers of __skb_flow_dissect.
    
    OTOH, in the filters above, cls_flower configures its key->basic.n_proto
    to the ETH_P_PPP_SES value configured by the user. Matching on this key
    fails because of __skb_flow_dissect "transparency" mentioned above.
    
    In the following, I would argue that the problem lies with cls_flower,
    unnessary attempting key->basic.n_proto match.
    
    There are 3 close places in fl_set_key in cls_flower setting up
    mask->basic.n_proto. They are (in reverse order of appearance in the
    code) due to:
    
    (a) No vlan is given: use TCA_FLOWER_KEY_ETH_TYPE parameter
    (b) One vlan tag is given: use TCA_FLOWER_KEY_VLAN_ETH_TYPE
    (c) Two vlans are given: use TCA_FLOWER_KEY_CVLAN_ETH_TYPE
    
    The match in case (a) is unneeded because flower has no its own
    eth_type parameter. It was removed by Jamal Hadi Salim in commit
    488b41d020fb06428b90289f70a41210718f52b7 in iproute2. For
    TCA_FLOWER_KEY_ETH_TYPE the userspace uses the generic tc filter
    protocol field. Therefore the match for the case (a) is done by tc
    itself.
    
    The matches in cases (b), (c) are unneeded because the protocol will
    appear in and will be matched by flow_dissector_key_vlan.vlan_tpid.
    Therefore in the best case, key->basic.n_proto will try to repeat vlan
    key match again.
    
    The below patch removes mask->basic.n_proto setting and resets it to 0
    in case (c).
    Signed-off-by: default avatarBoris Sukholitko <boris.sukholitko@broadcom.com>
    Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
    0dca2c74
cls_flower.c 92 KB