DPDK patches and discussions
 help / color / mirror / Atom feed
From: Dekel Peled <dekelp@nvidia.com>
To: Maxime Leroy <maxime.leroy@6wind.com>
Cc: Eli Britstein <elibr@mellanox.com>,
	"ferruh.yigit@intel.com" <ferruh.yigit@intel.com>,
	"arybchenko@solarflare.com" <arybchenko@solarflare.com>,
	Ori Kam <orika@mellanox.com>,
	"NBU-Contact-Thomas Monjalon" <thomas@monjalon.net>,
	Asaf Penso <asafp@mellanox.com>, "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [RFC] ethdev: add VLAN attributes to ETH item
Date: Mon, 7 Sep 2020 18:21:56 +0000
Message-ID: <MN2PR12MB4375E653F716BB5E0C5AC236BB280@MN2PR12MB4375.namprd12.prod.outlook.com> (raw)
In-Reply-To: <CAEykdvrxAtfLQ2TGoUkAiVppkaivOS-2NicYjdqPA3EDEckKKQ@mail.gmail.com>

Thanks, PSB.

> -----Original Message-----
> From: Maxime Leroy <maxime.leroy@6wind.com>
> Sent: Monday, September 7, 2020 7:13 PM
> To: Dekel Peled <dekelp@mellanox.com>
> Cc: Eli Britstein <elibr@mellanox.com>; ferruh.yigit@intel.com;
> arybchenko@solarflare.com; Ori Kam <orika@mellanox.com>; NBU-Contact-
> Thomas Monjalon <thomas@monjalon.net>; Asaf Penso
> <asafp@mellanox.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [RFC] ethdev: add VLAN attributes to ETH item
> 
> Hi Dekel,
> 
> First, I don't understand the initial change [1] done on RTE_FLOW API.

As [1] commit log specifies, it is meant to clarify the required pattern to use, and reduce ambiguity.

> Before this change, it was possible to match any packets with or without vlan
> encapsulations.
> At least, it's not anymore the case with the mlx5 pmd.
> 
> For example, if I want to match any ssh packets whatever if it's encapsulated
> with no vlan or N vlan headers:
> testpmd> flow create 0 ingress  pattern  eth type spec 0 type mask 0 /
> ipv4 / udp dst is 22  / end actions  mark id 2 / queue index 0 / end
> 
> By setting the ethernet type mask to 0x0, it means that ethernet type should
> be ignored. It means if ethernet type is 0x800 (i.e. ipv4) or
> 0x8100 (i.e. vlan) or 0x88A8 (qinq), the packet should be matched.
> 
> But if you wanted to match only ethernet packets (and not vlan/qinq one),
> you can create the following flows:
> testpmd> flow create 0 ingress  pattern  eth type spec 0x800 type mask
> 0xffff / ipv4 / udp dst is 22  / end actions  mark id 2 / queue index
> 0 / end
> 
> With your new RFC, first I don't understand the needs of the num_of_vlans
> field.

Actually v2 of this RFC was sent already, please refer to https://mails.dpdk.org/archives/dev/2020-August/177536.html.

> 
> You can create the following follow if you want to match any qinq /
> ipv4 packets (i.e. 2 vlan level) for example:
> > flow create 0 ingress  pattern  eth type spec 0x88A8 type mask 0xffff
> > / vlan inner_type spec is 0x8100 mask is 0xFFFF / vlan  / end actions
> > mark id 2 / queue index 0 / end
> 
> Could you explain to me the utility of this new field ?
> 
> The cvlan_exist, and svlan_exist seems useless for me. For me, you can
> already do the same thing with type field. Because, by setting the type mask
> to 0, you can already give the notion of any ethertype.
> 
> Let's take some example:
> 
> 1. with svlan_exists=0, cvlan_exists=0, it can already be configured like that:
> > flow  create 0 ingress  pattern  eth type spec 0x800 type mask 0xFFFF
> > / ipv4 / end actions  mark id 2 / queue index 0 / end
> 
> 2. With svlan_exists=0, cvlan_exists=1:
> > flow  create 0 ingress  pattern eth type spec 0x8100 type mask 0xFFFF
> > / vlan inner_type is 0x800 mask is 0xFFFF / ipv4 / end actions  mark
> > id 2 / queue index 0 / end
> 
> 3. With svlan_exists=1, cvlan_exists=0: it doesn't seem to be a real use case.
> Anyway, you could have:
> > flow create 0 ingress  pattern  eth type spec 0x88A8 type mask 0xffff
> > / vlan inner_type spec is 0x800 mask is 0xFFFF / ipv4  / end actions
> > mark id 2 / queue index 0 / end
> 
> 4. With svlan_exists=1, cvlan_exists=1:
>  > flow create 0 ingress  pattern  eth type spec 0x88A8 type mask 0xffff / vlan
> inner_type spec is 0x8100 mask is 0xFFFF / vlan spec is
> 0x800 mask 0xFFFF / ipv4 / end actions  mark id 2 / queue index 0 / end
> 
> Could you please explain to me what you try to achieve with this RFC ?
> 
> I would like to know why ether_type value setted by the user is ignored
> when I create the following rule:
> testpmd> flow create 0 ingress  pattern  eth type spec 0 type mask 0 /
> ipv4 / udp dst is 22  / end actions  mark id 2 / queue index 0 / end with the
> mlx5 pmd ? (i.e. why this change [1].)
> 
> [1]
> https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmails.
> dpdk.org%2Farchives%2Fdev%2F2020-
> May%2F166257.html&amp;data=02%7C01%7Cdekelp%40nvidia.com%7C7cb0
> 777ea0e841bdc19808d85348f520%7C43083d15727340c1b7db39efd9ccc17a%7
> C0%7C0%7C637350920106123048&amp;sdata=BWVQ0vSBMW2oDaGe0%2F6
> 6EsEY1z76jFVJLKKtrmTQHj0%3D&amp;reserved=0
> 
> Best Regards,
> 
> Maxime
> 
> On Wed, Aug 5, 2020 at 9:04 AM Matan Azrad <matan@mellanox.com>
> wrote:
> >
> > But if the user want to force only one vlan and don't care about others?
> >
> >
> > > -----Original Message-----
> > > From: Dekel Peled <dekelp@mellanox.com>
> > > Sent: Wednesday, August 5, 2020 9:54 AM
> > > To: Eli Britstein <elibr@mellanox.com>; ferruh.yigit@intel.com;
> > > arybchenko@solarflare.com; Ori Kam <orika@mellanox.com>; Thomas
> > > Monjalon <thomas@monjalon.net>
> > > Cc: Asaf Penso <asafp@mellanox.com>; Matan Azrad
> > > <matan@mellanox.com>; dev@dpdk.org
> > > Subject: RE: [RFC] ethdev: add VLAN attributes to ETH item
> > >
> > > Thanks, PSB.
> > >
> > > > -----Original Message-----
> > > > From: Eli Britstein <elibr@mellanox.com>
> > > > Sent: Tuesday, August 4, 2020 6:47 PM
> > > > To: Dekel Peled <dekelp@mellanox.com>; ferruh.yigit@intel.com;
> > > > arybchenko@solarflare.com; Ori Kam <orika@mellanox.com>; Thomas
> > > > Monjalon <thomas@monjalon.net>
> > > > Cc: Asaf Penso <asafp@mellanox.com>; Matan Azrad
> > > <matan@mellanox.com>;
> > > > dev@dpdk.org
> > > > Subject: Re: [RFC] ethdev: add VLAN attributes to ETH item
> > > >
> > > >
> > > > On 8/4/2020 6:36 PM, Dekel Peled wrote:
> > > > > In existing code the match on tagged/untagged packets is not explicit.
> > > > > Recent documentation update [1] describes the different patterns
> > > > > and clarifies the intended use of different patterns.
> > > > >
> > > > > This patch proposes an update to ETH item struct, to clearly
> > > > > define the required characteristic of a packet, and enable precise
> match criteria.
> > > > >
> > > > > [1]
> > > > >
> https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2
> > > > > Fmails.dpdk.org%2Farchives%2Fdev%2F2020-
> May%2F166257.html&amp;da
> > > > >
> ta=02%7C01%7Cdekelp%40nvidia.com%7C7cb0777ea0e841bdc19808d85348f
> > > > >
> 520%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637350920106123
> > > > >
> 048&amp;sdata=BWVQ0vSBMW2oDaGe0%2F66EsEY1z76jFVJLKKtrmTQHj0%
> 3D&a
> > > > > mp;reserved=0
> > > > >
> > > > > Signed-off-by: Dekel Peled <dekelp@mellanox.com>
> > > > > ---
> > > > >   lib/librte_ethdev/rte_flow.h | 9 +++++++++
> > > > >   1 file changed, 9 insertions(+)
> > > > >
> > > > > diff --git a/lib/librte_ethdev/rte_flow.h
> > > > > b/lib/librte_ethdev/rte_flow.h index cf0eccb..345feb5 100644
> > > > > --- a/lib/librte_ethdev/rte_flow.h
> > > > > +++ b/lib/librte_ethdev/rte_flow.h
> > > > > @@ -726,11 +726,20 @@ struct rte_flow_item_raw {
> > > > >    * If the @p ETH item is the only item in the pattern, and the
> > > > > @p type
> > > field
> > > > >    * is not specified, then both tagged and untagged packets
> > > > > will match
> > > the
> > > > >    * pattern.
> > > > > + * The fields @p cvlan_exist and @p svlan_exist can be used to
> > > > > + match specific
> > > > > + * packet types, instead of using the @p type field. This can
> > > > > + be used to match
> > > > > + * on packets that do/don't contain either cvlan, svlan, or both.
> > > > > + * The field @p num_of_vlans can be used to match packets by
> > > > > + the exact number
> > > > > + * of VLANs in header.
> > > > >    */
> > > > >   struct rte_flow_item_eth {
> > > > >           struct rte_ether_addr dst; /**< Destination MAC. */
> > > > >           struct rte_ether_addr src; /**< Source MAC. */
> > > > >           rte_be16_t type; /**< EtherType or TPID. */
> > > > > + uint32_t cvlan_exist:1; /**< C-tag VLAN exist in header. */
> > > > > + uint32_t svlan_exist:1; /**< S-tag VLAN exist in header. */
> > > > > + uint32_t reserved:14; /**< Reserved, must be zero. */ uint32_t
> > > > > + num_of_vlans:16; /**< Number of VLANs in header. */
> > > > We can deduct from num_of_vlans the values of
> > > > cvlan_exist/svlan_exist, so those are redundant fields. Keeping
> > > > them introduce a conflicting match. For example num_of_vlans=0 and
> cvlan_exist=1.
> > >
> > > Such conflict is simple to validate and reject.
> > > Even if num_of_vlans is removed, we can still get conflict
> > > svlan_exist=1, cvlan_exist=0.
> > > The different fields are proposed to allow flexible match on
> > > different VLAN attributes.
> > > Every PMD can choose to support any or none of them.
> > >
> > > > >   };
> > > > >
> > > > >   /** Default mask for RTE_FLOW_ITEM_TYPE_ETH. */

  reply	other threads:[~2020-09-07 18:22 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-04 15:36 Dekel Peled
2020-08-04 15:47 ` Eli Britstein
2020-08-05  6:53   ` Dekel Peled
2020-08-05  7:04     ` Matan Azrad
2020-09-07 16:13       ` Maxime Leroy
2020-09-07 18:21         ` Dekel Peled [this message]
2020-08-04 16:22 ` Stephen Hemminger
2020-08-05  6:48   ` Dekel Peled
2020-08-06 10:36 ` [dpdk-dev] [RFC v2] ethdev: add VLAN attributes to ETH and VLAN items Dekel Peled
2020-09-08  9:30   ` Maxime Leroy
2020-09-30 15:59     ` Matan Azrad

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=MN2PR12MB4375E653F716BB5E0C5AC236BB280@MN2PR12MB4375.namprd12.prod.outlook.com \
    --to=dekelp@nvidia.com \
    --cc=arybchenko@solarflare.com \
    --cc=asafp@mellanox.com \
    --cc=dev@dpdk.org \
    --cc=elibr@mellanox.com \
    --cc=ferruh.yigit@intel.com \
    --cc=maxime.leroy@6wind.com \
    --cc=orika@mellanox.com \
    --cc=thomas@monjalon.net \
    /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

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git