Message ID | 20220708122421.19309-3-marcin.szycik@linux.intel.com (mailing list archive) |
---|---|
State | Superseded |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | ice: PPPoE offload support | expand |
On Fri, Jul 08, 2022 at 02:24:19PM +0200, Marcin Szycik wrote: > From: Wojciech Drewek <wojciech.drewek@intel.com> > > Add support for PPPoE specific fields for tc-flower. > Those fields can be provided only when protocol was set > to ETH_P_PPP_SES. Defines, dump, load and set are being done here. > > Overwrite basic.n_proto only in case of PPP_IP and PPP_IPV6, ... and PPP_MPLS_UC/PPP_MPLS_MC in this new patch version. > otherwise leave it as ETH_P_PPP_SES. > > Signed-off-by: Wojciech Drewek <wojciech.drewek@intel.com> > --- > v4: > * support of MPLS inner fields > * session_id stored in __be16 > > include/uapi/linux/pkt_cls.h | 3 ++ > net/sched/cls_flower.c | 61 ++++++++++++++++++++++++++++++++++++ > 2 files changed, 64 insertions(+) > > diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h > index 9a2ee1e39fad..c142c0f8ed8a 100644 > --- a/include/uapi/linux/pkt_cls.h > +++ b/include/uapi/linux/pkt_cls.h > @@ -589,6 +589,9 @@ enum { > > TCA_FLOWER_KEY_NUM_OF_VLANS, /* u8 */ > > + TCA_FLOWER_KEY_PPPOE_SID, /* be16 */ > + TCA_FLOWER_KEY_PPP_PROTO, /* be16 */ > + > __TCA_FLOWER_MAX, > }; > > diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c > index dcca70144dff..2a0ebad0e61c 100644 > --- a/net/sched/cls_flower.c > +++ b/net/sched/cls_flower.c > @@ -16,6 +16,7 @@ > #include <linux/in6.h> > #include <linux/ip.h> > #include <linux/mpls.h> > +#include <linux/ppp_defs.h> > > #include <net/sch_generic.h> > #include <net/pkt_cls.h> > @@ -73,6 +74,7 @@ struct fl_flow_key { > struct flow_dissector_key_ct ct; > struct flow_dissector_key_hash hash; > struct flow_dissector_key_num_of_vlans num_of_vlans; > + struct flow_dissector_key_pppoe pppoe; > } __aligned(BITS_PER_LONG / 8); /* Ensure that we can do comparisons as longs. */ > > struct fl_flow_mask_range { > @@ -714,6 +716,8 @@ static const struct nla_policy fl_policy[TCA_FLOWER_MAX + 1] = { > [TCA_FLOWER_KEY_HASH] = { .type = NLA_U32 }, > [TCA_FLOWER_KEY_HASH_MASK] = { .type = NLA_U32 }, > [TCA_FLOWER_KEY_NUM_OF_VLANS] = { .type = NLA_U8 }, > + [TCA_FLOWER_KEY_PPPOE_SID] = { .type = NLA_U16 }, > + [TCA_FLOWER_KEY_PPP_PROTO] = { .type = NLA_U16 }, > > }; > > @@ -1041,6 +1045,50 @@ static void fl_set_key_vlan(struct nlattr **tb, > } > } > > +static void fl_set_key_pppoe(struct nlattr **tb, > + struct flow_dissector_key_pppoe *key_val, > + struct flow_dissector_key_pppoe *key_mask, > + struct fl_flow_key *key, > + struct fl_flow_key *mask) > +{ > + /* key_val::type must be set to ETH_P_PPP_SES > + * because ETH_P_PPP_SES was stored in basic.n_proto > + * which might get overwritten by ppp_proto > + * or might be set to 0, the role of key_val::type > + * is simmilar to vlan_key::tpid Didn't you mean "vlan_tpid"? > + */ > + key_val->type = htons(ETH_P_PPP_SES); > + key_mask->type = cpu_to_be16(~0); > + > + if (tb[TCA_FLOWER_KEY_PPPOE_SID]) { > + key_val->session_id = > + nla_get_be16(tb[TCA_FLOWER_KEY_PPPOE_SID]); > + key_mask->session_id = cpu_to_be16(~0); > + } > + if (tb[TCA_FLOWER_KEY_PPP_PROTO]) { > + key_val->ppp_proto = > + nla_get_be16(tb[TCA_FLOWER_KEY_PPP_PROTO]); > + key_mask->ppp_proto = cpu_to_be16(~0); > + > + if (key_val->ppp_proto == htons(PPP_IP)) { > + key->basic.n_proto = htons(ETH_P_IP); > + mask->basic.n_proto = cpu_to_be16(~0); > + } else if (key_val->ppp_proto == htons(PPP_IPV6)) { > + key->basic.n_proto = htons(ETH_P_IPV6); > + mask->basic.n_proto = cpu_to_be16(~0); > + } else if (key_val->ppp_proto == htons(PPP_MPLS_UC)) { > + key->basic.n_proto = htons(ETH_P_MPLS_UC); > + mask->basic.n_proto = cpu_to_be16(~0); > + } else if (key_val->ppp_proto == htons(PPP_MPLS_MC)) { > + key->basic.n_proto = htons(ETH_P_MPLS_MC); > + mask->basic.n_proto = cpu_to_be16(~0); > + } > + } else { > + key->basic.n_proto = 0; > + mask->basic.n_proto = cpu_to_be16(0); > + } > +} > + > static void fl_set_key_flag(u32 flower_key, u32 flower_mask, > u32 *dissector_key, u32 *dissector_mask, > u32 flower_flag_bit, u32 dissector_flag_bit) > @@ -1651,6 +1699,9 @@ static int fl_set_key(struct net *net, struct nlattr **tb, > } > } > > + if (key->basic.n_proto == htons(ETH_P_PPP_SES)) > + fl_set_key_pppoe(tb, &key->pppoe, &mask->pppoe, key, mask); > + > if (key->basic.n_proto == htons(ETH_P_IP) || > key->basic.n_proto == htons(ETH_P_IPV6)) { > fl_set_key_val(tb, &key->basic.ip_proto, TCA_FLOWER_KEY_IP_PROTO, > @@ -1923,6 +1974,8 @@ static void fl_init_dissector(struct flow_dissector *dissector, > FLOW_DISSECTOR_KEY_HASH, hash); > FL_KEY_SET_IF_MASKED(mask, keys, cnt, > FLOW_DISSECTOR_KEY_NUM_OF_VLANS, num_of_vlans); > + FL_KEY_SET_IF_MASKED(mask, keys, cnt, > + FLOW_DISSECTOR_KEY_PPPOE, pppoe); > > skb_flow_dissector_init(dissector, keys, cnt); > } > @@ -3051,6 +3104,14 @@ static int fl_dump_key(struct sk_buff *skb, struct net *net, > fl_dump_key_ip(skb, false, &key->ip, &mask->ip))) > goto nla_put_failure; > > + if (mask->pppoe.session_id && > + nla_put_be16(skb, TCA_FLOWER_KEY_PPPOE_SID, key->pppoe.session_id)) > + goto nla_put_failure; > + > + if (mask->basic.n_proto && mask->pppoe.ppp_proto && > + nla_put_be16(skb, TCA_FLOWER_KEY_PPP_PROTO, key->pppoe.ppp_proto)) > + goto nla_put_failure; > + > if (key->control.addr_type == FLOW_DISSECTOR_KEY_IPV4_ADDRS && > (fl_dump_key_val(skb, &key->ipv4.src, TCA_FLOWER_KEY_IPV4_SRC, > &mask->ipv4.src, TCA_FLOWER_KEY_IPV4_SRC_MASK, > -- > 2.35.1 >
> -----Original Message----- > From: Guillaume Nault <gnault@redhat.com> > Sent: piątek, 8 lipca 2022 21:23 > To: Marcin Szycik <marcin.szycik@linux.intel.com> > Cc: netdev@vger.kernel.org; Nguyen, Anthony L <anthony.l.nguyen@intel.com>; davem@davemloft.net; > xiyou.wangcong@gmail.com; Brandeburg, Jesse <jesse.brandeburg@intel.com>; gustavoars@kernel.org; > baowen.zheng@corigine.com; boris.sukholitko@broadcom.com; edumazet@google.com; kuba@kernel.org; jhs@mojatatu.com; > jiri@resnulli.us; kurt@linutronix.de; pablo@netfilter.org; pabeni@redhat.com; paulb@nvidia.com; simon.horman@corigine.com; > komachi.yoshiki@gmail.com; zhangkaiheb@126.com; intel-wired-lan@lists.osuosl.org; michal.swiatkowski@linux.intel.com; Drewek, > Wojciech <wojciech.drewek@intel.com>; Lobakin, Alexandr <alexandr.lobakin@intel.com>; mostrows@earthlink.net; > paulus@samba.org > Subject: Re: [RFC PATCH net-next v4 2/4] net/sched: flower: Add PPPoE filter > > On Fri, Jul 08, 2022 at 02:24:19PM +0200, Marcin Szycik wrote: > > From: Wojciech Drewek <wojciech.drewek@intel.com> > > > > Add support for PPPoE specific fields for tc-flower. > > Those fields can be provided only when protocol was set > > to ETH_P_PPP_SES. Defines, dump, load and set are being done here. > > > > Overwrite basic.n_proto only in case of PPP_IP and PPP_IPV6, > > ... and PPP_MPLS_UC/PPP_MPLS_MC in this new patch version. > > > otherwise leave it as ETH_P_PPP_SES. > > > > Signed-off-by: Wojciech Drewek <wojciech.drewek@intel.com> > > --- > > v4: > > * support of MPLS inner fields > > * session_id stored in __be16 > > > > include/uapi/linux/pkt_cls.h | 3 ++ > > net/sched/cls_flower.c | 61 ++++++++++++++++++++++++++++++++++++ > > 2 files changed, 64 insertions(+) > > > > diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h > > index 9a2ee1e39fad..c142c0f8ed8a 100644 > > --- a/include/uapi/linux/pkt_cls.h > > +++ b/include/uapi/linux/pkt_cls.h > > @@ -589,6 +589,9 @@ enum { > > > > TCA_FLOWER_KEY_NUM_OF_VLANS, /* u8 */ > > > > + TCA_FLOWER_KEY_PPPOE_SID, /* be16 */ > > + TCA_FLOWER_KEY_PPP_PROTO, /* be16 */ > > + > > __TCA_FLOWER_MAX, > > }; > > > > diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c > > index dcca70144dff..2a0ebad0e61c 100644 > > --- a/net/sched/cls_flower.c > > +++ b/net/sched/cls_flower.c > > @@ -16,6 +16,7 @@ > > #include <linux/in6.h> > > #include <linux/ip.h> > > #include <linux/mpls.h> > > +#include <linux/ppp_defs.h> > > > > #include <net/sch_generic.h> > > #include <net/pkt_cls.h> > > @@ -73,6 +74,7 @@ struct fl_flow_key { > > struct flow_dissector_key_ct ct; > > struct flow_dissector_key_hash hash; > > struct flow_dissector_key_num_of_vlans num_of_vlans; > > + struct flow_dissector_key_pppoe pppoe; > > } __aligned(BITS_PER_LONG / 8); /* Ensure that we can do comparisons as longs. */ > > > > struct fl_flow_mask_range { > > @@ -714,6 +716,8 @@ static const struct nla_policy fl_policy[TCA_FLOWER_MAX + 1] = { > > [TCA_FLOWER_KEY_HASH] = { .type = NLA_U32 }, > > [TCA_FLOWER_KEY_HASH_MASK] = { .type = NLA_U32 }, > > [TCA_FLOWER_KEY_NUM_OF_VLANS] = { .type = NLA_U8 }, > > + [TCA_FLOWER_KEY_PPPOE_SID] = { .type = NLA_U16 }, > > + [TCA_FLOWER_KEY_PPP_PROTO] = { .type = NLA_U16 }, > > > > }; > > > > @@ -1041,6 +1045,50 @@ static void fl_set_key_vlan(struct nlattr **tb, > > } > > } > > > > +static void fl_set_key_pppoe(struct nlattr **tb, > > + struct flow_dissector_key_pppoe *key_val, > > + struct flow_dissector_key_pppoe *key_mask, > > + struct fl_flow_key *key, > > + struct fl_flow_key *mask) > > +{ > > + /* key_val::type must be set to ETH_P_PPP_SES > > + * because ETH_P_PPP_SES was stored in basic.n_proto > > + * which might get overwritten by ppp_proto > > + * or might be set to 0, the role of key_val::type > > + * is simmilar to vlan_key::tpid > > Didn't you mean "vlan_tpid"? Yes, is vlan_key::tpid not clear/valid? > > > + */ > > + key_val->type = htons(ETH_P_PPP_SES); > > + key_mask->type = cpu_to_be16(~0); > > + > > + if (tb[TCA_FLOWER_KEY_PPPOE_SID]) { > > + key_val->session_id = > > + nla_get_be16(tb[TCA_FLOWER_KEY_PPPOE_SID]); > > + key_mask->session_id = cpu_to_be16(~0); > > + } > > + if (tb[TCA_FLOWER_KEY_PPP_PROTO]) { > > + key_val->ppp_proto = > > + nla_get_be16(tb[TCA_FLOWER_KEY_PPP_PROTO]); > > + key_mask->ppp_proto = cpu_to_be16(~0); > > + > > + if (key_val->ppp_proto == htons(PPP_IP)) { > > + key->basic.n_proto = htons(ETH_P_IP); > > + mask->basic.n_proto = cpu_to_be16(~0); > > + } else if (key_val->ppp_proto == htons(PPP_IPV6)) { > > + key->basic.n_proto = htons(ETH_P_IPV6); > > + mask->basic.n_proto = cpu_to_be16(~0); > > + } else if (key_val->ppp_proto == htons(PPP_MPLS_UC)) { > > + key->basic.n_proto = htons(ETH_P_MPLS_UC); > > + mask->basic.n_proto = cpu_to_be16(~0); > > + } else if (key_val->ppp_proto == htons(PPP_MPLS_MC)) { > > + key->basic.n_proto = htons(ETH_P_MPLS_MC); > > + mask->basic.n_proto = cpu_to_be16(~0); > > + } > > + } else { > > + key->basic.n_proto = 0; > > + mask->basic.n_proto = cpu_to_be16(0); > > + } > > +} > > + > > static void fl_set_key_flag(u32 flower_key, u32 flower_mask, > > u32 *dissector_key, u32 *dissector_mask, > > u32 flower_flag_bit, u32 dissector_flag_bit) > > @@ -1651,6 +1699,9 @@ static int fl_set_key(struct net *net, struct nlattr **tb, > > } > > } > > > > + if (key->basic.n_proto == htons(ETH_P_PPP_SES)) > > + fl_set_key_pppoe(tb, &key->pppoe, &mask->pppoe, key, mask); > > + > > if (key->basic.n_proto == htons(ETH_P_IP) || > > key->basic.n_proto == htons(ETH_P_IPV6)) { > > fl_set_key_val(tb, &key->basic.ip_proto, TCA_FLOWER_KEY_IP_PROTO, > > @@ -1923,6 +1974,8 @@ static void fl_init_dissector(struct flow_dissector *dissector, > > FLOW_DISSECTOR_KEY_HASH, hash); > > FL_KEY_SET_IF_MASKED(mask, keys, cnt, > > FLOW_DISSECTOR_KEY_NUM_OF_VLANS, num_of_vlans); > > + FL_KEY_SET_IF_MASKED(mask, keys, cnt, > > + FLOW_DISSECTOR_KEY_PPPOE, pppoe); > > > > skb_flow_dissector_init(dissector, keys, cnt); > > } > > @@ -3051,6 +3104,14 @@ static int fl_dump_key(struct sk_buff *skb, struct net *net, > > fl_dump_key_ip(skb, false, &key->ip, &mask->ip))) > > goto nla_put_failure; > > > > + if (mask->pppoe.session_id && > > + nla_put_be16(skb, TCA_FLOWER_KEY_PPPOE_SID, key->pppoe.session_id)) > > + goto nla_put_failure; > > + > > + if (mask->basic.n_proto && mask->pppoe.ppp_proto && > > + nla_put_be16(skb, TCA_FLOWER_KEY_PPP_PROTO, key->pppoe.ppp_proto)) > > + goto nla_put_failure; > > + > > if (key->control.addr_type == FLOW_DISSECTOR_KEY_IPV4_ADDRS && > > (fl_dump_key_val(skb, &key->ipv4.src, TCA_FLOWER_KEY_IPV4_SRC, > > &mask->ipv4.src, TCA_FLOWER_KEY_IPV4_SRC_MASK, > > -- > > 2.35.1 > >
On Mon, Jul 11, 2022 at 10:26:21AM +0000, Drewek, Wojciech wrote: > > > +static void fl_set_key_pppoe(struct nlattr **tb, > > > + struct flow_dissector_key_pppoe *key_val, > > > + struct flow_dissector_key_pppoe *key_mask, > > > + struct fl_flow_key *key, > > > + struct fl_flow_key *mask) > > > +{ > > > + /* key_val::type must be set to ETH_P_PPP_SES > > > + * because ETH_P_PPP_SES was stored in basic.n_proto > > > + * which might get overwritten by ppp_proto > > > + * or might be set to 0, the role of key_val::type > > > + * is simmilar to vlan_key::tpid > > > > Didn't you mean "vlan_tpid"? > > Yes, is vlan_key::tpid not clear/valid? At least it wasn't entirely clear to me as I wondered if I got the comment right. And it's basically free to use the real name of the structure field.
diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h index 9a2ee1e39fad..c142c0f8ed8a 100644 --- a/include/uapi/linux/pkt_cls.h +++ b/include/uapi/linux/pkt_cls.h @@ -589,6 +589,9 @@ enum { TCA_FLOWER_KEY_NUM_OF_VLANS, /* u8 */ + TCA_FLOWER_KEY_PPPOE_SID, /* be16 */ + TCA_FLOWER_KEY_PPP_PROTO, /* be16 */ + __TCA_FLOWER_MAX, }; diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c index dcca70144dff..2a0ebad0e61c 100644 --- a/net/sched/cls_flower.c +++ b/net/sched/cls_flower.c @@ -16,6 +16,7 @@ #include <linux/in6.h> #include <linux/ip.h> #include <linux/mpls.h> +#include <linux/ppp_defs.h> #include <net/sch_generic.h> #include <net/pkt_cls.h> @@ -73,6 +74,7 @@ struct fl_flow_key { struct flow_dissector_key_ct ct; struct flow_dissector_key_hash hash; struct flow_dissector_key_num_of_vlans num_of_vlans; + struct flow_dissector_key_pppoe pppoe; } __aligned(BITS_PER_LONG / 8); /* Ensure that we can do comparisons as longs. */ struct fl_flow_mask_range { @@ -714,6 +716,8 @@ static const struct nla_policy fl_policy[TCA_FLOWER_MAX + 1] = { [TCA_FLOWER_KEY_HASH] = { .type = NLA_U32 }, [TCA_FLOWER_KEY_HASH_MASK] = { .type = NLA_U32 }, [TCA_FLOWER_KEY_NUM_OF_VLANS] = { .type = NLA_U8 }, + [TCA_FLOWER_KEY_PPPOE_SID] = { .type = NLA_U16 }, + [TCA_FLOWER_KEY_PPP_PROTO] = { .type = NLA_U16 }, }; @@ -1041,6 +1045,50 @@ static void fl_set_key_vlan(struct nlattr **tb, } } +static void fl_set_key_pppoe(struct nlattr **tb, + struct flow_dissector_key_pppoe *key_val, + struct flow_dissector_key_pppoe *key_mask, + struct fl_flow_key *key, + struct fl_flow_key *mask) +{ + /* key_val::type must be set to ETH_P_PPP_SES + * because ETH_P_PPP_SES was stored in basic.n_proto + * which might get overwritten by ppp_proto + * or might be set to 0, the role of key_val::type + * is simmilar to vlan_key::tpid + */ + key_val->type = htons(ETH_P_PPP_SES); + key_mask->type = cpu_to_be16(~0); + + if (tb[TCA_FLOWER_KEY_PPPOE_SID]) { + key_val->session_id = + nla_get_be16(tb[TCA_FLOWER_KEY_PPPOE_SID]); + key_mask->session_id = cpu_to_be16(~0); + } + if (tb[TCA_FLOWER_KEY_PPP_PROTO]) { + key_val->ppp_proto = + nla_get_be16(tb[TCA_FLOWER_KEY_PPP_PROTO]); + key_mask->ppp_proto = cpu_to_be16(~0); + + if (key_val->ppp_proto == htons(PPP_IP)) { + key->basic.n_proto = htons(ETH_P_IP); + mask->basic.n_proto = cpu_to_be16(~0); + } else if (key_val->ppp_proto == htons(PPP_IPV6)) { + key->basic.n_proto = htons(ETH_P_IPV6); + mask->basic.n_proto = cpu_to_be16(~0); + } else if (key_val->ppp_proto == htons(PPP_MPLS_UC)) { + key->basic.n_proto = htons(ETH_P_MPLS_UC); + mask->basic.n_proto = cpu_to_be16(~0); + } else if (key_val->ppp_proto == htons(PPP_MPLS_MC)) { + key->basic.n_proto = htons(ETH_P_MPLS_MC); + mask->basic.n_proto = cpu_to_be16(~0); + } + } else { + key->basic.n_proto = 0; + mask->basic.n_proto = cpu_to_be16(0); + } +} + static void fl_set_key_flag(u32 flower_key, u32 flower_mask, u32 *dissector_key, u32 *dissector_mask, u32 flower_flag_bit, u32 dissector_flag_bit) @@ -1651,6 +1699,9 @@ static int fl_set_key(struct net *net, struct nlattr **tb, } } + if (key->basic.n_proto == htons(ETH_P_PPP_SES)) + fl_set_key_pppoe(tb, &key->pppoe, &mask->pppoe, key, mask); + if (key->basic.n_proto == htons(ETH_P_IP) || key->basic.n_proto == htons(ETH_P_IPV6)) { fl_set_key_val(tb, &key->basic.ip_proto, TCA_FLOWER_KEY_IP_PROTO, @@ -1923,6 +1974,8 @@ static void fl_init_dissector(struct flow_dissector *dissector, FLOW_DISSECTOR_KEY_HASH, hash); FL_KEY_SET_IF_MASKED(mask, keys, cnt, FLOW_DISSECTOR_KEY_NUM_OF_VLANS, num_of_vlans); + FL_KEY_SET_IF_MASKED(mask, keys, cnt, + FLOW_DISSECTOR_KEY_PPPOE, pppoe); skb_flow_dissector_init(dissector, keys, cnt); } @@ -3051,6 +3104,14 @@ static int fl_dump_key(struct sk_buff *skb, struct net *net, fl_dump_key_ip(skb, false, &key->ip, &mask->ip))) goto nla_put_failure; + if (mask->pppoe.session_id && + nla_put_be16(skb, TCA_FLOWER_KEY_PPPOE_SID, key->pppoe.session_id)) + goto nla_put_failure; + + if (mask->basic.n_proto && mask->pppoe.ppp_proto && + nla_put_be16(skb, TCA_FLOWER_KEY_PPP_PROTO, key->pppoe.ppp_proto)) + goto nla_put_failure; + if (key->control.addr_type == FLOW_DISSECTOR_KEY_IPV4_ADDRS && (fl_dump_key_val(skb, &key->ipv4.src, TCA_FLOWER_KEY_IPV4_SRC, &mask->ipv4.src, TCA_FLOWER_KEY_IPV4_SRC_MASK,