Re: [dpdk-dev] [RFC v2 1/2] ethdev: add packet filter flow and new behavior switch to fdir

["Wu, Jingjing" <jingjing.wu@intel.com>]

> 
> diff --git a/lib/librte_ether/rte_eth_ctrl.h b/lib/librte_ether/rte_eth_ctrl.h
> index ce224ad..5cc22a0 100644
> --- a/lib/librte_ether/rte_eth_ctrl.h
> +++ b/lib/librte_ether/rte_eth_ctrl.h
> @@ -74,7 +74,11 @@ extern "C" {
>  #define RTE_ETH_FLOW_IPV6_EX            15
>  #define RTE_ETH_FLOW_IPV6_TCP_EX        16
>  #define RTE_ETH_FLOW_IPV6_UDP_EX        17
> -#define RTE_ETH_FLOW_MAX                18
> +#define RTE_ETH_FLOW_PKT_FILTER_IPV4_TCP 18 #define
> +RTE_ETH_FLOW_PKT_FILTER_IPV4_UDP 19 #define
> +RTE_ETH_FLOW_PKT_FILTER_IPV6_TCP 20 #define
> +RTE_ETH_FLOW_PKT_FILTER_IPV6_UDP 21
> +#define RTE_ETH_FLOW_MAX                22
> 
How to distinguish RTE_ETH_FLOW_PKT_FILTER_IPV4_XX with RTE_ETH_FLOW_NONFRAG_IPV4_XX, what is the difference?
>  /**
>   * Feature filter types
> @@ -407,6 +411,9 @@ struct rte_eth_l2_flow {  struct rte_eth_ipv4_flow {
>  	uint32_t src_ip;      /**< IPv4 source address to match. */
>  	uint32_t dst_ip;      /**< IPv4 destination address to match. */
> +	uint8_t tos;          /**< IPV4 type of service to match. */
> +	uint8_t proto;        /**< IPV4 proto to match. */
> +	uint8_t ttl;          /**< IPV4 time to live to match. */
>  };
> 
>  /**
> @@ -443,6 +450,10 @@ struct rte_eth_sctpv4_flow {  struct
> rte_eth_ipv6_flow {
>  	uint32_t src_ip[4];      /**< IPv6 source address to match. */
>  	uint32_t dst_ip[4];      /**< IPv6 destination address to match. */
> +	uint8_t  tc;             /**< IPv6 traffic class to match. */
> +	uint32_t flow_label;     /**< IPv6 flow label to match. */
> +	uint8_t  next_header;    /**< IPv6 next header to match. */
> +	uint8_t  hop_limit;      /**< IPv6 hop limits to match. */
>  };
> 
There is also a patch http://dpdk.org/dev/patchwork/patch/9661/ which added these fields. Maybe we can merge them together.
> +struct rte_eth_pkt_filter_flow {
> +	enum rte_eth_pkt_filter_type type;   /**< Type of filter */
> +	enum rte_eth_pkt_filter_type prio;
> +	/**< Prioritize the filter type when a packet matches several types */
> +	struct rte_eth_pkt_filter pkt;      /**< Packet fields to match. */
> +	struct rte_eth_pkt_filter mask;     /**< Mask for matched fields. */
> +};
> +
> +/**
>   * An union contains the inputs for all types of flow
>   */
>  union rte_eth_fdir_flow {
> @@ -514,6 +570,7 @@ union rte_eth_fdir_flow {
>  	struct rte_eth_ipv6_flow   ipv6_flow;
>  	struct rte_eth_mac_vlan_flow mac_vlan_flow;
>  	struct rte_eth_tunnel_flow   tunnel_flow;
> +	struct rte_eth_pkt_filter_flow pkt_filter_flow;
>  };
Why not use rte_eth_XX_flow directly but add a new one? Is it because of the mask? If so, how about to add a field in rte_eth_fdir_input like:
struct rte_eth_fdir_input {
    uint16_t flow_type;
    union rte_eth_fdir_flow flow;
    /**< Flow fields to match, dependent on flow_type */
    union rte_eth_fdir_flow flow_mask;
    struct rte_eth_fdir_flow_ext flow_ext;
    /**< Additional fields to match */
};

Thanks
Jingjing