[dpdk-dev] [PATCH v2 1/3] net/i40e: add QinQ wrapper function

Iremonger, Bernard bernard.iremonger at intel.com
Tue Mar 28 15:23:28 CEST 2017


Hi Wenzhuo,

<snip>

-----Original Message-----
> > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Bernard
> > Iremonger
> > > Sent: Friday, March 24, 2017 12:39 AM
> > > To: dev at 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.



More information about the dev mailing list