On Thu, Nov 29, 2018 at 03:22:28AM +0100, 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.
> 
> This patch also includes support for FLOW_EXT and FLOW_MAC_EXT.
> 
> Signed-off-by: Pablo Neira Ayuso <pa...@netfilter.org>
...
> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
> index afd9596ce636..c76d1b34c9a2 100644
> --- a/include/linux/ethtool.h
> +++ b/include/linux/ethtool.h
...
> +
> +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;
> +     struct flow_dissector_key_vlan                  vlan;
> +     struct flow_dissector_key_eth_addrs             eth_addrs;
> +} __aligned(BITS_PER_LONG / 8); /* Ensure that we can do comparisons as 
> longs. */

How about __aligned(sizeof(long)) ?

...
> +     act = &flow->rule->action.entries[0];
> +     switch (fs->ring_cookie) {
> +     case RX_CLS_FLOW_DISC:
> +             act->id = FLOW_ACTION_DROP;
> +             break;
> +     case RX_CLS_FLOW_WAKE:
> +             act->id = FLOW_ACTION_WAKE;
> +             break;
> +     default:
> +             act->id = FLOW_ACTION_QUEUE;
> +             act->queue_index = fs->ring_cookie;
> +             break;
> +     }

IMHO it would be cleaner if rules passing packets to RSS contexts were
also implemented using action, e.g. by using FLOW_ACTION_CONTEXT for
act->id and reusing act->queue_index for context id (using either an
anonymous union or more generic name).

Another idea: I would prefer moving this translation from NIC drivers to
the generic ethtool code. That would mean providing new ethtool_ops
callbacks (with a plan to deprecate the old ones). But it's probably
something something to leave for later as there would be a lot of pain
for no actual gain right now. (The reason I'm thinking about this is
that it wouldn't feel right to translate netlink messages to the ethtool
structures only to translate those once again in the NIC driver.)

Michal Kubecek

Reply via email to