From mboxrd@z Thu Jan 1 00:00:00 1970 From: Florian Fainelli Subject: Re: [PATCH net-next,v3 09/12] flow_dissector: add basic ethtool_rx_flow_spec to flow_rule structure translator Date: Wed, 21 Nov 2018 20:57:31 -0800 Message-ID: <191049db-be77-811b-1c6d-59c5c9a2d251@gmail.com> References: <20181121025132.14305-1-pablo@netfilter.org> <20181121025132.14305-10-pablo@netfilter.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Cc: davem@davemloft.net, thomas.lendacky@amd.com, ariel.elior@cavium.com, michael.chan@broadcom.com, santosh@chelsio.com, madalin.bucur@nxp.com, yisen.zhuang@huawei.com, salil.mehta@huawei.com, jeffrey.t.kirsher@intel.com, tariqt@mellanox.com, saeedm@mellanox.com, jiri@mellanox.com, idosch@mellanox.com, jakub.kicinski@netronome.com, peppe.cavallaro@st.com, grygorii.strashko@ti.com, andrew@lunn.ch, vivien.didelot@savoirfairelinux.com, alexandre.torgue@st.com, joabreu@synopsys.com, linux-net-drivers@solarflare.com, ganeshgr@chelsio.com, ogerlitz@mellanox.com, Manish.Chopra@cavium.com To: Pablo Neira Ayuso , netdev@vger.kernel.org Return-path: Received: from mail-pl1-f196.google.com ([209.85.214.196]:38877 "EHLO mail-pl1-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2404557AbeKVPfY (ORCPT ); Thu, 22 Nov 2018 10:35:24 -0500 Received: by mail-pl1-f196.google.com with SMTP id e5so8625393plb.5 for ; Wed, 21 Nov 2018 20:57:45 -0800 (PST) In-Reply-To: <20181121025132.14305-10-pablo@netfilter.org> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 11/20/2018 6:51 PM, Pablo Neira Ayuso wrote: > This patch adds a function to translate the ethtool_rx_flow_spec > structure to the flow_rule representation. > > This allows us to reuse code from the driver side given that both flower > and ethtool_rx_flow interfaces use the same representation. > > Signed-off-by: Pablo Neira Ayuso > --- > v3: Suggested by Jiri Pirko: > - Add struct ethtool_rx_flow_rule, keep placeholder to private > dissector information. > Reported by Manish Chopra: > - Fix incorrect dissector user_keys flags. > > include/linux/ethtool.h | 10 +++ > net/core/ethtool.c | 189 ++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 199 insertions(+) > > diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h > index afd9596ce636..99849e0858b2 100644 > --- a/include/linux/ethtool.h > +++ b/include/linux/ethtool.h > @@ -400,4 +400,14 @@ struct ethtool_ops { > void (*get_ethtool_phy_stats)(struct net_device *, > struct ethtool_stats *, u64 *); > }; > + > +struct ethtool_rx_flow_rule { > + struct flow_rule *rule; > + unsigned long priv[0]; > +}; > + > +struct ethtool_rx_flow_rule * > +ethtool_rx_flow_rule_alloc(const struct ethtool_rx_flow_spec *fs); > +void ethtool_rx_flow_rule_free(struct ethtool_rx_flow_rule *rule); > + > #endif /* _LINUX_ETHTOOL_H */ > diff --git a/net/core/ethtool.c b/net/core/ethtool.c > index d05402868575..e679d6478371 100644 > --- a/net/core/ethtool.c > +++ b/net/core/ethtool.c > @@ -28,6 +28,7 @@ > #include > #include > #include > +#include > > /* > * Some useful ethtool_ops methods that're device independent. > @@ -2808,3 +2809,191 @@ int dev_ethtool(struct net *net, struct ifreq *ifr) > > return rc; > } > + > +struct ethtool_rx_flow_key { > + struct flow_dissector_key_basic basic; > + union { > + struct flow_dissector_key_ipv4_addrs ipv4; > + struct flow_dissector_key_ipv6_addrs ipv6; > + }; > + struct flow_dissector_key_ports tp; > + struct flow_dissector_key_ip ip; > +} __aligned(BITS_PER_LONG / 8); /* Ensure that we can do comparisons as longs. */ > + > +struct ethtool_rx_flow_match { > + struct flow_dissector dissector; > + struct ethtool_rx_flow_key key; > + struct ethtool_rx_flow_key mask; > +}; > + > +struct ethtool_rx_flow_rule * > +ethtool_rx_flow_rule_alloc(const struct ethtool_rx_flow_spec *fs) This is more than alloc, it's allocate and map, no reason to split the two operations AFAICT, but the name could be improved, how about alloc_from()? > +{ > + static struct in6_addr zero_addr = {}; > + struct ethtool_rx_flow_match *match; > + struct ethtool_rx_flow_rule *flow; > + struct flow_action_entry *act; > + > + flow = kzalloc(sizeof(struct ethtool_rx_flow_rule) + > + sizeof(struct ethtool_rx_flow_match), GFP_KERNEL); > + if (!flow) > + return NULL; > + > + /* ethtool_rx supports only one single action per rule. */ > + flow->rule = flow_rule_alloc(1); > + if (!flow->rule) { > + kfree(flow); > + return NULL; > + } > + > + match = (struct ethtool_rx_flow_match *)flow->priv; > + flow->rule->match.dissector = &match->dissector; > + flow->rule->match.mask = &match->mask; > + flow->rule->match.key = &match->key; > + > + match->mask.basic.n_proto = 0xffff; > + > + switch (fs->flow_type & ~FLOW_EXT) { > + case TCP_V4_FLOW: > + case UDP_V4_FLOW: { > + const struct ethtool_tcpip4_spec *v4_spec, *v4_m_spec; > + > + match->key.basic.n_proto = htons(ETH_P_IP); > + > + v4_spec = &fs->h_u.tcp_ip4_spec; > + v4_m_spec = &fs->m_u.tcp_ip4_spec; > + > + if (v4_m_spec->ip4src) { > + match->key.ipv4.src = v4_spec->ip4src; > + match->mask.ipv4.src = v4_m_spec->ip4src; > + } > + if (v4_m_spec->ip4dst) { > + match->key.ipv4.dst = v4_spec->ip4dst; > + match->mask.ipv4.dst = v4_m_spec->ip4dst; > + } I got confused a while ago between the ethtool ntuple and nfc semantics, and I can't remember if the following is true: - bits set to 1 indicate a match and bit set to 0 indicate a don't care for nfc - bits set to 0 indicate a match and bit set to 1 indicate a don't care for ntuple Depending on the answer that could mean that this check on a zero address may have to change. > + if (v4_m_spec->ip4src || > + v4_m_spec->ip4dst) { > + match->dissector.used_keys |= > + (1 << FLOW_DISSECTOR_KEY_IPV4_ADDRS); Can you use BIT() here (and likewise for every one below). [snip] > + > + return flow; What about the extended fields and non-IP protocols? -- Florian