Hi Wenzhuo, <snip>
-----Original Message----- > > > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Bernard > > Iremonger > > > Sent: Friday, March 24, 2017 12:39 AM > > > To: dev@dpdk.org; Xing, Beilei; Wu, Jingjing > > > Cc: Zhang, Helin; Iremonger, Bernard; Stroe, Laura > > > Subject: [dpdk-dev] [PATCH v2 1/3] net/i40e: add QinQ wrapper > > > function > > > > > > Add i40e_dev_cloud_filter_qinq function, and call it from > > > i40e_dev_consistent_tunnel_filter_set function. > > > Replace filter 0x1 with QinQ filter. > > Better not use 0x1 here. Maybe it should be change to outer IP filter? > Ok, I will change to out IP filter. > > And seems better to add more explanation about what's QinQ filter here. > Ok, I will add some information. > > > > > + > > > +/* Create a QinQ cloud filter > > > + * > > > + * The Fortville NIC has limited resources for tunnel filters, > > > + * so we can only reuse existing filters. > > > + * > > > + * In step 1 we define which Field Vector fields can be used for > > > + * filter types. > > > + * As we do not have the inner tag defined as a field, > > > + * we have to define it first, by reusing one of L1 entries. > > > + * > > > + * In step 2 we are replacing one of existing filter types with > > > + * a new one for QinQ. > > > + * As we reusing L1 and replacing L2, some of the default filter > > > + * types will disappear,which depends on L1 and L2 entries we reuse. > > > + * > > > + * Step 1: Create L1 filter of outer vlan (12b) + inner vlan (12b) > > > + * > > > + * 1. Create L1 filter of outer vlan (12b) which will be in use > > > + * later when we define the cloud filter. > > > + * a. Valid_flags.replace_cloud = 0 > > > + * b. Old_filter = 10 (Stag_Inner_Vlan) > > > + * c. New_filter = 0x10 > > > + * d. TR bit = 0xff (optional, not used here) > > > + * e. Buffer – 2 entries: > > > + * i. Byte0 = 8 (outer vlan FV index). Byte1 =0 (rsv) > > > Byte 2- > > > 3 = 0x0fff > > > + * ii. Byte0 = 37 (inner vlan FV index). Byte1 =0 > > > (rsv) Byte > > > 2-3 = 0x0fff > > > + * > > > + * Step 2: > > > + * 2. Create cloud filter using two L1 filters entries: stag and > > > + * new filter(outer vlan+ inner vlan) > > > + * a. Valid_flags.replace_cloud = 1 > > > + * b. Old_filter = 1 (instead of outer IP) > > > + * c. New_filter = 0x10 > > > + * d. Buffer – 2 entries: > > > + * i. Byte0 = 0x80 | 7 (valid | Stag). Byte13 = 0 > > > (rsv) > > > + * ii. Byte8 = 0x80 | 0x10 (valid | new l1 filter > > > step1). > > > Byte9-11 = 0 (rsv) > > > + */ > > > +static int > > > +i40e_dev_cloud_filter_qinq(struct i40e_pf *pf) { > > Is it better to change the function name to > > i40e_dev_create_cloud_filter_qinq? Ok, how about i40e_cloud_filter_qinq_create()? It is more in line with the existing code. > > > > > diff --git a/drivers/net/i40e/i40e_ethdev.h > > > b/drivers/net/i40e/i40e_ethdev.h index 934c679..020c5a2 100644 > > > --- a/drivers/net/i40e/i40e_ethdev.h > > > +++ b/drivers/net/i40e/i40e_ethdev.h > > > > > > /** > > > + * filter type of tunneling packet > > > + */ > > > +#define ETH_TUNNEL_FILTER_OMAC 0x01 /**< filter by outer MAC > addr > > */ > > > +#define ETH_TUNNEL_FILTER_OIP 0x02 /**< filter by outer IP Addr */ > > > +#define ETH_TUNNEL_FILTER_TENID 0x04 /**< filter by tenant ID */ > > > +#define ETH_TUNNEL_FILTER_IMAC 0x08 /**< filter by inner MAC addr > > */ > > > +#define ETH_TUNNEL_FILTER_IVLAN 0x10 /**< filter by inner VLAN ID > */ > > > +#define ETH_TUNNEL_FILTER_IIP 0x20 /**< filter by inner IP addr */ > > Seems not necessary to redefine these macros. > > > > > +#define ETH_TUNNEL_FILTER_OVLAN 0x40 /**< filter by outer VLAN ID > > */ > > > + > > > +#define I40E_TUNNEL_FILTER_IMAC_IVLAN > (ETH_TUNNEL_FILTER_IMAC > > | \ > > > + ETH_TUNNEL_FILTER_IVLAN) > > > +#define I40E_TUNNEL_FILTER_IMAC_IVLAN_TENID > > > (ETH_TUNNEL_FILTER_IMAC | \ > > > + ETH_TUNNEL_FILTER_IVLAN | \ > > > + ETH_TUNNEL_FILTER_TENID) > > > +#define I40E_TUNNEL_FILTER_IMAC_TENID > (ETH_TUNNEL_FILTER_IMAC > > | \ > > > + ETH_TUNNEL_FILTER_TENID) > > > +#define I40E_TUNNEL_FILTER_OMAC_TENID_IMAC > > > (ETH_TUNNEL_FILTER_OMAC | \ > > > + ETH_TUNNEL_FILTER_TENID | \ > > > + ETH_TUNNEL_FILTER_IMAC) > > I don't think it's necessary the redefine these macros either. Maybe > > we only need to add I40E_TUNNEL_FILTER_CUSTOM_QINQ. > > I will check this. These are now used in the i40e_ethdev code replacing RTE_* macros. > > > > > +#define I40E_TUNNEL_FILTER_CUSTOM_QINQ > > (ETH_TUNNEL_FILTER_OMAC > > > | \ > > > + ETH_TUNNEL_FILTER_OVLAN | > > > ETH_TUNNEL_FILTER_IVLAN) > > > + > > > +/** > > > * Tunneling Packet filter configuration. > > > */ > > > struct i40e_tunnel_filter_conf { > > > -- > > > 2.10.1 > > Thanks for the review. > > Regards, > > Bernard.
