DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ori Kam <orika@mellanox.com>
To: "Sexton, Rory" <rory.sexton@intel.com>, "dev@dpdk.org" <dev@dpdk.org>
Cc: "Zhang, Qi Z" <qi.z.zhang@intel.com>,
	"Xing, Beilei" <beilei.xing@intel.com>,
	Adrien Mazarguil <adrien.mazarguil@6wind.com>,
	"Jagus, DariuszX" <dariuszx.jagus@intel.com>
Subject: Re: [dpdk-dev] [PATCH] ethdev: add L2TPv3 header to flow API
Date: Tue, 10 Dec 2019 20:32:19 +0000	[thread overview]
Message-ID: <AM4PR05MB3425A0FDA21BA88C2DF0A0F3DB5B0@AM4PR05MB3425.eurprd05.prod.outlook.com> (raw)
In-Reply-To: <MN2PR11MB3678F4058CCFC25143B89D83EC5B0@MN2PR11MB3678.namprd11.prod.outlook.com>

Hi Rory,

> -----Original Message-----
> From: Sexton, Rory <rory.sexton@intel.com>
> Sent: Tuesday, December 10, 2019 4:52 PM
> To: Ori Kam <orika@mellanox.com>; dev@dpdk.org
> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; Xing, Beilei <beilei.xing@intel.com>;
> Adrien Mazarguil <adrien.mazarguil@6wind.com>; Jagus, DariuszX
> <dariuszx.jagus@intel.com>
> Subject: RE: [dpdk-dev] [PATCH] ethdev: add L2TPv3 header to flow API
> 
> Hi Ori,
> 
> > One general question why do we want to support only v3 and not also v2?
> l2tpv3 is more widely used as a tunneling protocol hence it's inclusion.
> A specific example is the cable industry where DOCSIS cable traffic is
> encapsulated using depi and uepi protocols which both make use of l2tpv3
> session ids.
> Directing flows based on l2tpv3 can be very useful in these cases.
> 

I'm not saying that v3 is not used more, I just thought from looking at the spec
that both can be supported and the only difference is the version, so matching on
the version we will be able to support both versions.
Your decision.

> Some more comments inline below.
> 
> Rory
> 
> >> diff --git a/lib/librte_ethdev/rte_flow.h
> >> b/lib/librte_ethdev/rte_flow.h index 452d359a1..5ee055c28 100644
> >> --- a/lib/librte_ethdev/rte_flow.h
> >> +++ b/lib/librte_ethdev/rte_flow.h
> >> @@ -510,6 +510,16 @@ enum rte_flow_item_type {
> >>  	 * See struct rte_flow_item_tag.
> >>  	 */
> >>  	RTE_FLOW_ITEM_TYPE_TAG,
> >> +
> >> +	/*
> >
> >Missing *. It should be /**
> >
> 
> Will correct this in another version of this patch.
> 
> >> +/**
> >> + * RTE_FLOW_ITEM_TYPE_L2TPV3.
> >> + *
> >> + * Matches a L2TPv3 header.
> >> + */
> >> +struct rte_flow_item_l2tpv3 {
> >> +	rte_be32_t session_id; /**< Session ID. */ };
> >
> >Does it make sense that in future we will want to match on the T / L / ver /
> Ns / Nr?
> >
> >Please also consider that any change will be ABI / API breakage, which will
> be allowed only next year.
> >
> 
> I don't foresee us wanting to match on any of the other fields, T / L / ver /
> Ns/ Nr.
> The session id would typically be the only field of interest in the l2tpv3
> header.

I think that adding all fields to the structure will solve the possible issue with adding matching later.
Also and even more important you will be able to use it for creating the  raw_encap / raw_decap buffers.

What do you think?

Best,
Ori

  reply	other threads:[~2019-12-10 20:32 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-04 14:10 Rory Sexton
2019-12-04 14:10 ` [dpdk-dev] [PATCH] net/i40e: Add new customized pctype for l2tpv3 Rory Sexton
2019-12-11 22:51   ` Xing, Beilei
2019-12-13 11:17     ` Sexton, Rory
2019-12-13 17:33       ` Xing, Beilei
2019-12-16 10:13         ` Sexton, Rory
2019-12-10 10:16 ` [dpdk-dev] [PATCH] ethdev: add L2TPv3 header to flow API Ori Kam
2019-12-10 14:52   ` Sexton, Rory
2019-12-10 20:32     ` Ori Kam [this message]
2019-12-11 11:36       ` Sexton, Rory
2019-12-11 13:30         ` Ori Kam
2019-12-11 16:31           ` Sexton, Rory
2019-12-12 13:38             ` Sexton, Rory

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=AM4PR05MB3425A0FDA21BA88C2DF0A0F3DB5B0@AM4PR05MB3425.eurprd05.prod.outlook.com \
    --to=orika@mellanox.com \
    --cc=adrien.mazarguil@6wind.com \
    --cc=beilei.xing@intel.com \
    --cc=dariuszx.jagus@intel.com \
    --cc=dev@dpdk.org \
    --cc=qi.z.zhang@intel.com \
    --cc=rory.sexton@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).