DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Iremonger, Bernard" <bernard.iremonger@intel.com>
To: "Lu, Wenzhuo" <wenzhuo.lu@intel.com>,
	"dev@dpdk.org" <dev@dpdk.org>,
	"Xing, Beilei" <beilei.xing@intel.com>,
	"Wu, Jingjing" <jingjing.wu@intel.com>
Cc: "Zhang, Helin" <helin.zhang@intel.com>,
	"Stroe, Laura" <laura.stroe@intel.com>
Subject: Re: [dpdk-dev] [PATCH v2 1/3] net/i40e: add QinQ wrapper function
Date: Wed, 29 Mar 2017 09:11:16 +0000	[thread overview]
Message-ID: <8CEF83825BEC744B83065625E567D7C224D4C433@IRSMSX108.ger.corp.intel.com> (raw)
In-Reply-To: <6A0DE07E22DDAD4C9103DF62FEBC09093B583F29@shsmsx102.ccr.corp.intel.com>

Hi Wenzhuo,

> -----Original Message-----
> From: Lu, Wenzhuo
> Sent: Wednesday, March 29, 2017 1:53 AM
> To: Iremonger, Bernard <bernard.iremonger@intel.com>; dev@dpdk.org;
> Xing, Beilei <beilei.xing@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>
> Cc: Zhang, Helin <helin.zhang@intel.com>; Stroe, Laura
> <laura.stroe@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v2 1/3] net/i40e: add QinQ wrapper function
> 
> Hi Bernard,
> 
> > -----Original Message-----
> > From: Iremonger, Bernard
> > Sent: Tuesday, March 28, 2017 9:23 PM
> > To: Iremonger, Bernard; Lu, Wenzhuo; dev@dpdk.org; Xing, Beilei; Wu,
> > Jingjing
> > Cc: Zhang, Helin; Stroe, Laura
> > Subject: RE: [dpdk-dev] [PATCH v2 1/3] net/i40e: add QinQ wrapper
> > function
> >
> > Hi Wenzhuo,
> >
> > <snip>
> >
> > -----Original Message-----
> > > > > From: dev [mailto:dev-bounces@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.
> It's fine to me:)
> 
> >
> > > >
> > > > > 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.
> I think these macros are not NIC specific. Better define them in RTE. But if it's
> not the good time to add them to RTE, maybe the concern of ABI change. We
> need only define ETH_TUNNEL_FILTER_OVLAN and
> I40E_TUNNEL_FILTER_OMAC_TENID_IMAC here.

If I just define the above filters, then the following switch statement has mixed RTE and I40E macros, it seems cleaner to me to use all  I40E macros. I can change back to mixed macros if you think it is better.

static int
i40e_dev_get_filter_type(uint16_t filter_type, uint16_t *flag)
{
	switch (filter_type) {
	case I40E_TUNNEL_FILTER_IMAC_IVLAN:
		*flag = I40E_AQC_ADD_CLOUD_FILTER_IMAC_IVLAN;
		break;
	case I40E_TUNNEL_FILTER_IMAC_IVLAN_TENID:
		*flag = I40E_AQC_ADD_CLOUD_FILTER_IMAC_IVLAN_TEN_ID;
		break;
	case I40E_TUNNEL_FILTER_CUSTOM_QINQ:
		*flag = I40E_AQC_ADD_CLOUD_FILTER_CUSTOM_QINQ;
		break;
	case I40E_TUNNEL_FILTER_IMAC_TENID:
		*flag = I40E_AQC_ADD_CLOUD_FILTER_IMAC_TEN_ID;
		break;
	case I40E_TUNNEL_FILTER_OMAC_TENID_IMAC:
		*flag = I40E_AQC_ADD_CLOUD_FILTER_OMAC_TEN_ID_IMAC;
		break;
	case ETH_TUNNEL_FILTER_IMAC:
		*flag = I40E_AQC_ADD_CLOUD_FILTER_IMAC;
		break;
	case ETH_TUNNEL_FILTER_OIP:
		*flag = I40E_AQC_ADD_CLOUD_FILTER_OIP;
		break;
	case ETH_TUNNEL_FILTER_IIP:
		*flag = I40E_AQC_ADD_CLOUD_FILTER_IIP;
		break;
	default:
		PMD_DRV_LOG(ERR, "invalid tunnel filter type");
		return -EINVAL;
	}

	return 0;
> 
> >
> > > >
> > > > > +#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.

Regards,

Bernard.


  reply	other threads:[~2017-03-29  9:11 UTC|newest]

Thread overview: 74+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-03 14:48 [dpdk-dev] [PATCH v1 0/4] net/i40e: QinQ filter Bernard Iremonger
2017-03-03 14:48 ` [dpdk-dev] [PATCH v1 1/4] net/i40e: update destroy tunnel filter function Bernard Iremonger
2017-03-03 14:48 ` [dpdk-dev] [PATCH v1 2/4] net/i40e: add QinQ wrapper function Bernard Iremonger
2017-03-03 14:48 ` [dpdk-dev] [PATCH v1 3/4] net/i40e: changes to consistent filter set Bernard Iremonger
2017-03-03 14:48 ` [dpdk-dev] [PATCH v1 4/4] net/i40e: parse QinQ pattern Bernard Iremonger
2017-03-23 16:38 ` [dpdk-dev] [PATCH v2 0/3] net/i40e: QinQ filter Bernard Iremonger
2017-03-28 16:20   ` [dpdk-dev] [PATCH v3 0/5] " Bernard Iremonger
2017-03-29 16:41     ` [dpdk-dev] [PATCH v4 " Bernard Iremonger
2017-03-30 16:01       ` Bernard Iremonger
2017-03-30 16:09         ` [dpdk-dev] [PATCH v5 " Bernard Iremonger
2017-03-31 13:33           ` Ferruh Yigit
2017-03-31 13:37             ` Iremonger, Bernard
2017-03-31 15:18           ` [dpdk-dev] [PATCH v6 " Bernard Iremonger
2017-03-31 16:53             ` Ferruh Yigit
2017-03-31 17:05               ` Iremonger, Bernard
2017-03-31 17:58             ` [dpdk-dev] [PATCH v7 " Bernard Iremonger
2017-04-03  9:24               ` Ferruh Yigit
2017-03-31 17:58             ` [dpdk-dev] [PATCH v7 1/5] net/i40e: initialise L3 MAP register Bernard Iremonger
2017-03-31 17:58             ` [dpdk-dev] [PATCH v7 2/5] net/i40e: add QinQ filter create function Bernard Iremonger
2017-03-31 17:58             ` [dpdk-dev] [PATCH v7 3/5] net/i40e: parse QinQ pattern Bernard Iremonger
2017-03-31 17:58             ` [dpdk-dev] [PATCH v7 4/5] net/i40e: update destroy tunnel filter function Bernard Iremonger
2017-03-31 17:58             ` [dpdk-dev] [PATCH v7 5/5] net/i40e: update tunnel filter restore function Bernard Iremonger
2017-03-31 15:18           ` [dpdk-dev] [PATCH v6 1/5] net/i40e: initialise L3 MAP register Bernard Iremonger
2017-03-31 15:18           ` [dpdk-dev] [PATCH v6 2/5] net/i40e: add QinQ filter create function Bernard Iremonger
2017-03-31 15:18           ` [dpdk-dev] [PATCH v6 3/5] net/i40e: parse QinQ pattern Bernard Iremonger
2017-03-31 15:18           ` [dpdk-dev] [PATCH v6 4/5] net/i40e: update destroy tunnel filter function Bernard Iremonger
2017-03-31 15:18           ` [dpdk-dev] [PATCH v6 5/5] net/i40e: update tunnel filter restore function Bernard Iremonger
2017-03-30 16:09         ` [dpdk-dev] [PATCH v5 1/5] net/i40e: initialise L3 MAP register Bernard Iremonger
2017-03-30 16:09         ` [dpdk-dev] [PATCH v5 2/5] net/i40e: add QinQ filter create function Bernard Iremonger
2017-03-31  9:43           ` Xing, Beilei
2017-03-30 16:09         ` [dpdk-dev] [PATCH v5 3/5] net/i40e: parse QinQ pattern Bernard Iremonger
2017-03-31  0:58           ` Lu, Wenzhuo
2017-03-31  8:01             ` Iremonger, Bernard
2017-03-31  8:27               ` Lu, Wenzhuo
2017-03-30 16:09         ` [dpdk-dev] [PATCH v5 4/5] net/i40e: update destroy tunnel filter function Bernard Iremonger
2017-03-30 16:09         ` [dpdk-dev] [PATCH v5 5/5] net/i40e: update tunnel filter restore function Bernard Iremonger
2017-03-30 16:01       ` [dpdk-dev] [PATCH v4 1/5] net/i40e: initialise L3 MAP register Bernard Iremonger
2017-03-30 16:01       ` [dpdk-dev] [PATCH v4 2/5] net/i40e: add QinQ filter create function Bernard Iremonger
2017-03-30 16:01       ` [dpdk-dev] [PATCH v4 3/5] net/i40e: parse QinQ pattern Bernard Iremonger
2017-03-30 16:01       ` [dpdk-dev] [PATCH v4 4/5] net/i40e: update destroy tunnel filter function Bernard Iremonger
2017-03-30 16:01       ` [dpdk-dev] [PATCH v4 5/5] net/i40e: update tunnel filter restore function Bernard Iremonger
2017-03-29 16:41     ` [dpdk-dev] [PATCH v4 1/5] net/i40e: initialise L3 MAP register Bernard Iremonger
2017-03-30  0:54       ` Lu, Wenzhuo
2017-03-29 16:41     ` [dpdk-dev] [PATCH v4 2/5] net/i40e: add QinQ filter create function Bernard Iremonger
2017-03-30  0:57       ` Lu, Wenzhuo
2017-03-29 16:41     ` [dpdk-dev] [PATCH v4 3/5] net/i40e: parse QinQ pattern Bernard Iremonger
2017-03-29 16:41     ` [dpdk-dev] [PATCH v4 4/5] net/i40e: update destroy tunnel filter function Bernard Iremonger
2017-03-29 16:41     ` [dpdk-dev] [PATCH v4 5/5] net/i40e: update tunnel filter restore function Bernard Iremonger
2017-03-28 16:20   ` [dpdk-dev] [PATCH v3 1/5] net/i40e: add QinQ filter create function Bernard Iremonger
2017-03-28 16:20   ` [dpdk-dev] [PATCH v3 2/5] net/i40e: parse QinQ pattern Bernard Iremonger
2017-03-29  1:25     ` Lu, Wenzhuo
2017-03-29 15:10       ` Iremonger, Bernard
2017-03-29  2:46     ` Xing, Beilei
2017-03-29  6:16       ` Xing, Beilei
2017-03-28 16:20   ` [dpdk-dev] [PATCH v3 3/5] net/i40e: update destroy tunnel filter function Bernard Iremonger
2017-03-28 16:20   ` [dpdk-dev] [PATCH v3 4/5] net/i40e: initialise L3 MAP register Bernard Iremonger
2017-03-29  1:24     ` Xing, Beilei
2017-03-29  8:27       ` Iremonger, Bernard
2017-03-29  1:53     ` Lu, Wenzhuo
2017-03-29  8:56       ` Iremonger, Bernard
2017-03-28 16:20   ` [dpdk-dev] [PATCH v3 5/5] net/i40e: update tunnel filter restore function Bernard Iremonger
2017-03-29  1:54     ` Lu, Wenzhuo
2017-03-23 16:38 ` [dpdk-dev] [PATCH v2 1/3] net/i40e: add QinQ wrapper function Bernard Iremonger
2017-03-28  8:11   ` Lu, Wenzhuo
2017-03-28 11:09     ` Iremonger, Bernard
2017-03-28 13:23       ` Iremonger, Bernard
2017-03-29  0:52         ` Lu, Wenzhuo
2017-03-29  9:11           ` Iremonger, Bernard [this message]
2017-03-23 16:38 ` [dpdk-dev] [PATCH v2 2/3] net/i40e: parse QinQ pattern Bernard Iremonger
2017-03-28  8:45   ` Lu, Wenzhuo
2017-03-28 11:12     ` Iremonger, Bernard
2017-03-23 16:38 ` [dpdk-dev] [PATCH v2 3/3] net/i40e: update destroy tunnel filter function Bernard Iremonger
2017-03-28  8:54   ` Lu, Wenzhuo
2017-03-28 11:13     ` Iremonger, Bernard

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8CEF83825BEC744B83065625E567D7C224D4C433@IRSMSX108.ger.corp.intel.com \
    --to=bernard.iremonger@intel.com \
    --cc=beilei.xing@intel.com \
    --cc=dev@dpdk.org \
    --cc=helin.zhang@intel.com \
    --cc=jingjing.wu@intel.com \
    --cc=laura.stroe@intel.com \
    --cc=wenzhuo.lu@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).