Commit 2105f700 authored by Vlad Buslov's avatar Vlad Buslov Committed by David S. Miller

net/sched: flower: fix parsing of ethertype following VLAN header

A tc flower filter matching TCA_FLOWER_KEY_VLAN_ETH_TYPE is expected to
match the L2 ethertype following the first VLAN header, as confirmed by
linked discussion with the maintainer. However, such rule also matches
packets that have additional second VLAN header, even though filter has
both eth_type and vlan_ethtype set to "ipv4". Looking at the code this
seems to be mostly an artifact of the way flower uses flow dissector.
First, even though looking at the uAPI eth_type and vlan_ethtype appear
like a distinct fields, in flower they are all mapped to the same
key->basic.n_proto. Second, flow dissector skips following VLAN header as
no keys for FLOW_DISSECTOR_KEY_CVLAN are set and eventually assigns the
value of n_proto to last parsed header. With these, such filters ignore any
headers present between first VLAN header and first "non magic"
header (ipv4 in this case) that doesn't result
FLOW_DISSECT_RET_PROTO_AGAIN.

Fix the issue by extending flow dissector VLAN key structure with new
'vlan_eth_type' field that matches first ethertype following previously
parsed VLAN header. Modify flower classifier to set the new
flow_dissector_key_vlan->vlan_eth_type with value obtained from
TCA_FLOWER_KEY_VLAN_ETH_TYPE/TCA_FLOWER_KEY_CVLAN_ETH_TYPE uAPIs.

Link: https://lore.kernel.org/all/Yjhgi48BpTGh6dig@nanopsycho/
Fixes: 9399ae9a ("net_sched: flower: Add vlan support")
Fixes: d64efd09 ("net/sched: flower: Add supprt for matching on QinQ vlan headers")
Signed-off-by: default avatarVlad Buslov <vladbu@nvidia.com>
Reviewed-by: default avatarJiri Pirko <jiri@nvidia.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 5a80059d
...@@ -59,6 +59,8 @@ struct flow_dissector_key_vlan { ...@@ -59,6 +59,8 @@ struct flow_dissector_key_vlan {
__be16 vlan_tci; __be16 vlan_tci;
}; };
__be16 vlan_tpid; __be16 vlan_tpid;
__be16 vlan_eth_type;
u16 padding;
}; };
struct flow_dissector_mpls_lse { struct flow_dissector_mpls_lse {
......
...@@ -1183,6 +1183,7 @@ bool __skb_flow_dissect(const struct net *net, ...@@ -1183,6 +1183,7 @@ bool __skb_flow_dissect(const struct net *net,
VLAN_PRIO_MASK) >> VLAN_PRIO_SHIFT; VLAN_PRIO_MASK) >> VLAN_PRIO_SHIFT;
} }
key_vlan->vlan_tpid = saved_vlan_tpid; key_vlan->vlan_tpid = saved_vlan_tpid;
key_vlan->vlan_eth_type = proto;
} }
fdret = FLOW_DISSECT_RET_PROTO_AGAIN; fdret = FLOW_DISSECT_RET_PROTO_AGAIN;
......
...@@ -1013,6 +1013,7 @@ static int fl_set_key_mpls(struct nlattr **tb, ...@@ -1013,6 +1013,7 @@ static int fl_set_key_mpls(struct nlattr **tb,
static void fl_set_key_vlan(struct nlattr **tb, static void fl_set_key_vlan(struct nlattr **tb,
__be16 ethertype, __be16 ethertype,
int vlan_id_key, int vlan_prio_key, int vlan_id_key, int vlan_prio_key,
int vlan_next_eth_type_key,
struct flow_dissector_key_vlan *key_val, struct flow_dissector_key_vlan *key_val,
struct flow_dissector_key_vlan *key_mask) struct flow_dissector_key_vlan *key_mask)
{ {
...@@ -1031,6 +1032,11 @@ static void fl_set_key_vlan(struct nlattr **tb, ...@@ -1031,6 +1032,11 @@ static void fl_set_key_vlan(struct nlattr **tb,
} }
key_val->vlan_tpid = ethertype; key_val->vlan_tpid = ethertype;
key_mask->vlan_tpid = cpu_to_be16(~0); key_mask->vlan_tpid = cpu_to_be16(~0);
if (tb[vlan_next_eth_type_key]) {
key_val->vlan_eth_type =
nla_get_be16(tb[vlan_next_eth_type_key]);
key_mask->vlan_eth_type = cpu_to_be16(~0);
}
} }
static void fl_set_key_flag(u32 flower_key, u32 flower_mask, static void fl_set_key_flag(u32 flower_key, u32 flower_mask,
...@@ -1602,8 +1608,9 @@ static int fl_set_key(struct net *net, struct nlattr **tb, ...@@ -1602,8 +1608,9 @@ static int fl_set_key(struct net *net, struct nlattr **tb,
if (eth_type_vlan(ethertype)) { if (eth_type_vlan(ethertype)) {
fl_set_key_vlan(tb, ethertype, TCA_FLOWER_KEY_VLAN_ID, fl_set_key_vlan(tb, ethertype, TCA_FLOWER_KEY_VLAN_ID,
TCA_FLOWER_KEY_VLAN_PRIO, &key->vlan, TCA_FLOWER_KEY_VLAN_PRIO,
&mask->vlan); TCA_FLOWER_KEY_VLAN_ETH_TYPE,
&key->vlan, &mask->vlan);
if (tb[TCA_FLOWER_KEY_VLAN_ETH_TYPE]) { if (tb[TCA_FLOWER_KEY_VLAN_ETH_TYPE]) {
ethertype = nla_get_be16(tb[TCA_FLOWER_KEY_VLAN_ETH_TYPE]); ethertype = nla_get_be16(tb[TCA_FLOWER_KEY_VLAN_ETH_TYPE]);
...@@ -1611,6 +1618,7 @@ static int fl_set_key(struct net *net, struct nlattr **tb, ...@@ -1611,6 +1618,7 @@ static int fl_set_key(struct net *net, struct nlattr **tb,
fl_set_key_vlan(tb, ethertype, fl_set_key_vlan(tb, ethertype,
TCA_FLOWER_KEY_CVLAN_ID, TCA_FLOWER_KEY_CVLAN_ID,
TCA_FLOWER_KEY_CVLAN_PRIO, TCA_FLOWER_KEY_CVLAN_PRIO,
TCA_FLOWER_KEY_CVLAN_ETH_TYPE,
&key->cvlan, &mask->cvlan); &key->cvlan, &mask->cvlan);
fl_set_key_val(tb, &key->basic.n_proto, fl_set_key_val(tb, &key->basic.n_proto,
TCA_FLOWER_KEY_CVLAN_ETH_TYPE, TCA_FLOWER_KEY_CVLAN_ETH_TYPE,
...@@ -3002,13 +3010,13 @@ static int fl_dump_key(struct sk_buff *skb, struct net *net, ...@@ -3002,13 +3010,13 @@ static int fl_dump_key(struct sk_buff *skb, struct net *net,
goto nla_put_failure; goto nla_put_failure;
if (mask->basic.n_proto) { if (mask->basic.n_proto) {
if (mask->cvlan.vlan_tpid) { if (mask->cvlan.vlan_eth_type) {
if (nla_put_be16(skb, TCA_FLOWER_KEY_CVLAN_ETH_TYPE, if (nla_put_be16(skb, TCA_FLOWER_KEY_CVLAN_ETH_TYPE,
key->basic.n_proto)) key->basic.n_proto))
goto nla_put_failure; goto nla_put_failure;
} else if (mask->vlan.vlan_tpid) { } else if (mask->vlan.vlan_eth_type) {
if (nla_put_be16(skb, TCA_FLOWER_KEY_VLAN_ETH_TYPE, if (nla_put_be16(skb, TCA_FLOWER_KEY_VLAN_ETH_TYPE,
key->basic.n_proto)) key->vlan.vlan_eth_type))
goto nla_put_failure; goto nla_put_failure;
} }
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment