Re: [dpdk-dev] [PATCH v4 07/12] librte_ether: extend flow director struct

[Thomas Monjalon <thomas.monjalon@6wind.com>]

Hi Jingjing,

2016-03-10 11:25, Jingjing Wu:
> This patch changed rte_eth_fdir_flow from union to struct to
> support more packets formats, for example, Vxlan and GRE tunnel
> packets with IP inner frame.

I think we need a lot more explanations about how it should work.
>From this point we should collect some acknowledgements from the
maintainers of other drivers having this kind of flow steering need.
Maybe that a better API, more generic, is possible.

> This patch also add new RTE_FDIR_TUNNEL_TYPE_GRE enum.

OK to add GRE to the existing API.

> Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
> Acked-by: Helin Zhang <helin.zhang@intel.com>
[...]
>  /**
> - * An union contains the inputs for all types of flow
> + * A struct contains the inputs for all types of flow
>   */
> -union rte_eth_fdir_flow {
> -	struct rte_eth_l2_flow     l2_flow;
> -	struct rte_eth_udpv4_flow  udp4_flow;
> -	struct rte_eth_tcpv4_flow  tcp4_flow;
> -	struct rte_eth_sctpv4_flow sctp4_flow;
> -	struct rte_eth_ipv4_flow   ip4_flow;
> -	struct rte_eth_udpv6_flow  udp6_flow;
> -	struct rte_eth_tcpv6_flow  tcp6_flow;
> -	struct rte_eth_sctpv6_flow sctp6_flow;
> -	struct rte_eth_ipv6_flow   ipv6_flow;
> +struct rte_eth_fdir_flow {
> +	union {
> +		struct rte_eth_l2_flow     l2_flow;
> +		struct rte_eth_udpv4_flow  udp4_flow;
> +		struct rte_eth_tcpv4_flow  tcp4_flow;
> +		struct rte_eth_sctpv4_flow sctp4_flow;
> +		struct rte_eth_ipv4_flow   ip4_flow;
> +		struct rte_eth_udpv6_flow  udp6_flow;
> +		struct rte_eth_tcpv6_flow  tcp6_flow;
> +		struct rte_eth_sctpv6_flow sctp6_flow;
> +		struct rte_eth_ipv6_flow   ipv6_flow;
> +	};
>  	struct rte_eth_mac_vlan_flow mac_vlan_flow;
>  	struct rte_eth_tunnel_flow   tunnel_flow;
>  };

Please explain somewhere how to use this API change in order to have more
discussions with other maintainers.

I'm sorry to comment this change only now. I took time to realize that
we need more consensus about the filtering API to make it usable by
more drivers.

For the 16.04 release, I suggest to remove this change from the series.
Thanks for your understanding.