DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Zhang, Qi Z" <qi.z.zhang@intel.com>
To: Thomas Monjalon <thomas@monjalon.net>,
	"Yigit, Ferruh" <ferruh.yigit@intel.com>,
	"Guo, Jia" <jia.guo@intel.com>
Cc: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>,
	Ori Kam <orika@nvidia.com>,
	"Wu, Jingjing" <jingjing.wu@intel.com>,
	"Yang, Qiming" <qiming.yang@intel.com>,
	"Wang, Haiyue" <haiyue.wang@intel.com>,
	"dev@dpdk.org" <dev@dpdk.org>,
	Gregory Etelson <getelson@nvidia.com>,
	"maxime.coquelin@redhat.com" <maxime.coquelin@redhat.com>,
	"jerinj@marvell.com" <jerinj@marvell.com>,
	"ajit.khaparde@broadcom.com" <ajit.khaparde@broadcom.com>
Subject: Re: [dpdk-dev] [dpdk-dev v2 1/2] ethdev: add new tunnel type for ecpri
Date: Mon, 11 Jan 2021 14:02:43 +0000
Message-ID: <c619e74fd486481c874dc39104817091@intel.com> (raw)
In-Reply-To: <9702007.LWHD5a8EkP@thomas>



> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Monday, January 11, 2021 7:38 PM
> To: Yigit, Ferruh <ferruh.yigit@intel.com>; Guo, Jia <jia.guo@intel.com>; Zhang,
> Qi Z <qi.z.zhang@intel.com>
> Cc: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>; Ori Kam
> <orika@nvidia.com>; Wu, Jingjing <jingjing.wu@intel.com>; Yang, Qiming
> <qiming.yang@intel.com>; Wang, Haiyue <haiyue.wang@intel.com>;
> dev@dpdk.org; Gregory Etelson <getelson@nvidia.com>;
> maxime.coquelin@redhat.com; jerinj@marvell.com;
> ajit.khaparde@broadcom.com
> Subject: Re: [dpdk-dev] [dpdk-dev v2 1/2] ethdev: add new tunnel type for ecpri
> 
> 11/01/2021 12:26, Zhang, Qi Z:
> > From: Thomas Monjalon <thomas@monjalon.net>
> > > 10/01/2021 11:46, Ori Kam:
> > > > From: Zhang, Qi Z <qi.z.zhang@intel.com>
> > > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > > 08/01/2021 10:29, Andrew Rybchenko:
> > > > > > > On 1/8/21 11:57 AM, Ferruh Yigit wrote:
> > > > > > > > On 1/8/2021 1:41 AM, Zhang, Qi Z wrote:
> > > > > > > >> From: Thomas Monjalon <thomas@monjalon.net>
> > > > > > > >>> 07/01/2021 16:24, Zhang, Qi Z:
> > > > > > > >>>> From: Thomas Monjalon <thomas@monjalon.net>
> > > > > > > >>>>> 07/01/2021 13:47, Zhang, Qi Z:
> > > > > > > >>>>>> From: Thomas Monjalon <thomas@monjalon.net>
> > > > > > > >>>>>>> 07/01/2021 10:32, Guo, Jia:
> > > > > > > >>>>>>>> From: Thomas Monjalon <thomas@monjalon.net>
> > > > > > > >>>>>>>>> 24/12/2020 07:59, Jeff Guo:
> > > > > > > >>>>>>>>>> --- a/lib/librte_ethdev/rte_ethdev.h
> > > > > > > >>>>>>>>>> +++ b/lib/librte_ethdev/rte_ethdev.h
> > > > > > > >>>>>>>>>> @@ -1219,6 +1219,7 @@ enum rte_eth_tunnel_type {
> > > > > > > >>>>>>>>>>       RTE_TUNNEL_TYPE_IP_IN_GRE,
> > > > > > > >>>>>>>>>>       RTE_L2_TUNNEL_TYPE_E_TAG,
> > > > > > > >>>>>>>>>>       RTE_TUNNEL_TYPE_VXLAN_GPE,
> > > > > > > >>>>>>>>>> +    RTE_TUNNEL_TYPE_ECPRI,
> > > > > > > >>>>>>>>>>       RTE_TUNNEL_TYPE_MAX,
> > > > > > > >>>>>>>>>>   };
> > > > > > > >>>>>>>>>
> > > > > > > >>>>>>>>> We tried to remove all these legacy API in DPDK 20.11.
> > > > > > > >>>>>>>>> Andrew decided to not remove this one because it
> > > > > > > >>>>>>>>> is not yet completely replaced by rte_flow in all drivers.
> > > > > > > >>>>>>>>> However, I am against continuing to update this API.
> > > > > > > >>>>>>>>> The opposite work should be done: migrate to rte_flow.
> > > > > > > >>>>>>>>
> > > > > > > >>>>>>>> Agree but seems that the legacy api and driver
> > > > > > > >>>>>>>> legacy implementation still keep in this release,
> > > > > > > >>>>>>>> and there is no a general way to replace the legacy by
> rte_flow right now.
> > > > > > > >>>>>>>
> > > > > > > >>>>>>> I think rte_flow is a complete replacement with more
> features.
> > > > > > > >>>>>>
> > > > > > > >>>>>> Thomas, I may not agree with this.
> > > > > > > >>>>>>
> > > > > > > >>>>>> Actually the "enum rte_eth_tunnel_type" is used by
> > > > > > > >>>>>> rte_eth_dev_udp_tunnel_port_add A packet with
> > > > > > > >>>>>> specific dst udp port will be recognized as a
> > > > > > > >>>>>> specific tunnel packet type
> > > (e.g.
> > > > > > > >>>>>> vxlan, vxlan-gpe,
> > > > > > > >>>>> ecpri...) In Intel NIC, the API actually changes the
> > > > > > > >>>>> configuration of the packet parser in HW but not add a
> > > > > > > >>>>> filter rule and I guess all other devices may enable
> > > > > > > >>>>> it in a similar
> > > way.
> > > > > > > >>>>>> so naturally it should be a device (port) level
> > > > > > > >>>>>> configuration but not a rte_flow
> > > > > > > >>>>> rule for match, encap, decap...
> > > > > > > >>>>>
> > > > > > > >>>>> I don't understand how it helps to identify an UDP
> > > > > > > >>>>> port if there is no rule for this tunnel.
> > > > > > > >>>>> What is the usage?
> > > > > > > >>>>
> > > > > > > >>>> Yes, in general It is a rule, it matches a udp packet's
> > > > > > > >>>> dst port and the action is
> > > > > > > >>> "now the packet is identified as vxlan packet" then all
> > > > > > > >>> other rte_flow rules that match for a vlxan as pattern
> > > > > > > >>> will take
> > > effect.
> > > > > > > >>> but somehow, I think they are not rules in the same
> > > > > > > >>> domain, just like we have dedicate API for mac/vlan
> > > > > > > >>> filter, we'd better have a dedicate API for this also. (
> > > > > > > >>> RFC for Vxlan explains why we need this.
> > > > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%
> > > > > 2Fto
> > > > > ols.ietf
> > > > > .org%2Fhtml%2Frfc7348&amp;data=04%7C01%7Corika%40nvidia.com
> %7C
> > > 46b2
> > > > >
> > >
> d8f48944422f0d9008d8b43a2293%7C43083d15727340c1b7db39efd9ccc17a
> > > %7
> > > > >
> > >
> C0%7C0%7C637457509081543237%7CUnknown%7CTWFpbGZsb3d8eyJWIjoi
> > > MC
> > > > >
> > >
> 4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&a
> > > > >
> > >
> mp;sdata=RYWFMjuxkcUZ982kK2s44tBAjf%2FTkDyaa7VEybCtxOo%3D&amp;r
> > > es
> > > > > erved=0).
> > > > > > > >>>>
> > > > > > > >>>> "Destination Port: IANA has assigned the value 4789 for
> > > > > > > >>>> the VXLAN UDP port, and this value SHOULD be used by
> > > > > > > >>>> default as the destination UDP port.  Some early
> > > > > > > >>>> implementations of VXLAN have used other values for the
> > > > > > > >>>> destination port.  To enable interoperability with
> > > > > > > >>>> these implementations, the destination port
> > > > > > SHOULD be configurable."
> > > > > > > >>>
> > > > > > > >>> Yes the port number is free.
> > > > > > > >>> But isn't it more natural to specify this port number as
> > > > > > > >>> part of the rte_flow rule?
> > > > > > > >>
> > > > > > > >> I think if we have a rte_flow action type that can be
> > > > > > > >> used to set a packet's tunnel type xxx, like below #flow
> > > > > > > >> create eth/ipv4/udp port is 4789/... action
> > > > > > > >> set_tunnel_type VxLAN / end then we may replace it with
> > > > > > > >> rte_flow, but I'm not sure if it's necessary, please share if you have
> a better idea.
> > > > > >
> > > > > > Of course we can specify the UDP port in rte_flow rule.
> > > > > > Please check rte_flow_item_udp.
> > > > > > That's a basic of rte_flow.
> > > > >
> > > > > Its not about the pattern match, it's about the action, what we
> > > > > need is a rte_flow action to "define a packet's tunnel type", but we don't
> have.
> > >
> > > A packet type alone is meaningless.
> > > It is always associated to an action, this is what rte_flow does.
> >
> > As I mentioned in previous, this is a device (port) level configuration, so it can
> only be configured by a PF driver or a privileged VF base on our security model.
> > A typical usage in a NFV environment could be:
> >
> > 1. A privileged VF (e.g. ice_dcf PMD) use rte_eth_dev_udp_tunnel_port_add
> to create tunnel port for eCPRI, them this will impact on all VFs in the same PF.
> > 2. A normal VF driver can create rte_flow rule that match specific patch for
> queue steering or apply RSS for eCPRI packets, but it DON'T have the
> permission to define the tunnel port.
> 
> Whaooh! A normal Intel VF is not allowed to match the tunnel it wants if not
> enabled by a priviledged VF?

> I would say it is a HW design flaw, but that's not the question.	

Why you think this is a design flaw? in real case, is it a typical requirement that different VF need different tunnel port for eCPRI (or VxLan) on the same PF? 
I believe it's not necessary to make it as a per VF resource in most cases, and I will be surprise if a driver that allow any VF to change the share resource without any privilege control.

Btw I guess mlx NIC has more flexible way to handle ecpri tunnel, just curious how it works, what's the expected result of below rules?

1. create flow eth / ipv4 / udp dst is 1234 / ecpri msgtype is 0 / ... to queue 0
2. create flow eth / ipv4 / udp dst is 5678 / ecrpi msgtype is 1 / ... to queue 1.

So both 1234 and 5678 will be regarded as an ECPRI packet? Or only the first one will work?
does dst udp port is always needed if an ecpri pattern is involved?


> 
> > So it does not help to have a rte_flow that match dst udp port as
> > tunnel while have an action in the same rule.
> 
> Now I understand you are truly looking for a device configuration.
> But as it looks more as a HW limitation, I don't think it should be part of ethdev.
> The fact is that it is already part of ethdev...
> 
> I don't know what to say. I need to digest how Intel limitation is "still" trying to
> drive some parts of the "generic" API.
> 
> 
> > > > > #flow create eth/ipv4/udp port is 4789/... action
> > > > > set_tunnel_type VxLAN
> > > > >
> > > > > I think rte_eth_dev_udp_tunnel_port_add does this job well
> > > > > already, if we plan to move it to rte_flow, at least we need a replacement
> solution.
> > >
> > > The documentation does not say why it is useful.
> > > With rte_flow you don't need it because a flow is specified with its action.
> > >
> > >
> > > > Let me see if I understand it correctly.
> > > > In your case, the issue is that you need to configure the HW to
> > > > parse the
> > > packet correctly right?
> > > > It is not about the matching it is about the configuration of the
> > > > HW, you wish to tell the HW that the packet should be parsed by
> > > > different means
> > > correct?
> > > >
> > > > If this is the case it sounds to me that you should use rte_flow
> > > > and if the user adds the following rule:
> > > > #flow create pattern eth / ivp4 / udp port is 4789 / .. action .....
> > > > You simply need to configure your HW to support the ecpri configuration.
> > > >
> > > >
> > > > > >
> > > > > >
> > > > > > > > Isn't this more a device configuration than filtering, not
> > > > > > > > sure about using rte_flow for this.
> > > > > > >
> > > > > > > +1
> > > > > >
> > > > > > A device configuration? No, setting an UDP port is a stack
> configuration.
> > > > > >
> > > > > >
> > > > > > > >> BTW, are we going to move all other filter like mac ,
> > > > > > > >> VLAN filter/strip/insert into rte_flow finally?
> > > > > >
> > > > > > Yes I think it should be the direction.
> > > > > > All of this can be achieved with rte_flow.
> > > > > >
> > > > > >
> > > > > > > >> if that's the plan, though I don't have much inputs for
> > > > > > > >> this right now, but I think we may not need to prevent
> > > > > > > >> new features be added based on current API if it does not
> > > > > > > >> introduce more complexity and not break anything.
> > > > > >
> > > > > > If we continue updating old API, we are just forking ourself:
> > > > > > having 2 APIs for the same feature is a non-sense.
> > > > > > We must remove APIs which are superseded by rte_flow.
> 
> 


  reply	other threads:[~2021-01-11 14:04 UTC|newest]

Thread overview: 91+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-16  8:58 [dpdk-dev] [dpdk-dev 21.02 0/5] enable UDP ecpri configure in dcf Jeff Guo
2020-12-16  8:58 ` [dpdk-dev] [dpdk-dev 21.02 1/5] ethdev: add new tunnel type for ecpri Jeff Guo
2020-12-23  9:28   ` Zhang, Qi Z
2020-12-16  8:58 ` [dpdk-dev] [dpdk-dev 21.02 2/5] net/ice/base: add new UDP " Jeff Guo
2020-12-16  8:58 ` [dpdk-dev] [dpdk-dev 21.02 3/5] net/ice: add ecpri package type Jeff Guo
2020-12-16  8:58 ` [dpdk-dev] [dpdk-dev 21.02 4/5] net/ice: enable ecpri tunnel port configure in dcf Jeff Guo
2020-12-16  8:58 ` [dpdk-dev] [dpdk-dev 21.02 5/5] app/testpmd: add new UDP tunnel port for ecpri Jeff Guo
2020-12-24  6:59 ` [dpdk-dev] [dpdk-dev v2 0/2] " Jeff Guo
2020-12-24  6:59   ` [dpdk-dev] [dpdk-dev v2 1/2] ethdev: add new tunnel type " Jeff Guo
2021-01-06 22:12     ` Thomas Monjalon
2021-01-07  9:32       ` Guo, Jia
2021-01-07 10:09         ` Andrew Rybchenko
2021-01-07 10:11         ` Thomas Monjalon
2021-01-07 12:47           ` Zhang, Qi Z
2021-01-07 13:33             ` Thomas Monjalon
2021-01-07 13:45               ` David Marchand
2021-01-07 14:27                 ` Dodji Seketeli
2021-01-07 15:24               ` Zhang, Qi Z
2021-01-07 16:58                 ` Thomas Monjalon
2021-01-08  1:41                   ` Zhang, Qi Z
2021-01-08  8:57                     ` Ferruh Yigit
2021-01-08  9:29                       ` Andrew Rybchenko
2021-01-08 10:36                         ` Thomas Monjalon
2021-01-09  1:01                           ` Zhang, Qi Z
2021-01-10 10:46                             ` Ori Kam
2021-01-11  9:23                               ` Thomas Monjalon
2021-01-11 11:26                                 ` Zhang, Qi Z
2021-01-11 11:37                                   ` Thomas Monjalon
2021-01-11 14:02                                     ` Zhang, Qi Z [this message]
2021-01-11 14:53                                       ` Thomas Monjalon
2021-01-12  2:14                                         ` Zhang, Qi Z
2021-01-15 15:15                                           ` Thomas Monjalon
2021-01-08  9:22               ` Ferruh Yigit
2021-01-08 10:23                 ` Thomas Monjalon
2021-01-08 10:43                   ` Ferruh Yigit
2021-01-08 14:06                     ` Thomas Monjalon
2021-01-08 14:07                       ` Kinsella, Ray
2021-01-08 14:10                         ` Thomas Monjalon
2021-01-08 12:38                   ` Kinsella, Ray
2021-01-08 14:27                     ` Ferruh Yigit
2021-01-08 14:31                       ` Kinsella, Ray
2021-01-08 17:34                       ` Kinsella, Ray
2021-01-14 14:34     ` Ferruh Yigit
2021-01-15  2:51       ` Guo, Jia
2020-12-24  6:59   ` [dpdk-dev] [dpdk-dev v2 2/2] app/testpmd: add new UDP tunnel port " Jeff Guo
2021-01-14 14:33     ` Ferruh Yigit
2021-01-15  2:13       ` Guo, Jia
2020-12-24 13:40   ` [dpdk-dev] [dpdk-dev v2 0/2] " Zhang, Qi Z
2020-12-24  7:01 ` [dpdk-dev] [dpdk-dev v2 0/6] enable UDP ecpri configure in dcf Jeff Guo
2020-12-24  7:01   ` [dpdk-dev] [dpdk-dev v2 1/6] net/ice/base: add package PTYPE enable information Jeff Guo
2020-12-24  7:01   ` [dpdk-dev] [dpdk-dev v2 2/6] net/ice: refactor package type parsing Jeff Guo
2020-12-24  7:01   ` [dpdk-dev] [dpdk-dev v2 3/6] net/ice/base: add new UDP tunnel type for ecpri Jeff Guo
2020-12-24  7:01   ` [dpdk-dev] [dpdk-dev v2 4/6] net/ice: add PTYPE mapping " Jeff Guo
2020-12-24  7:01   ` [dpdk-dev] [dpdk-dev v2 5/6] net/iavf: " Jeff Guo
2020-12-24  7:01   ` [dpdk-dev] [dpdk-dev v2 6/6] net/ice: enable ecpri tunnel port configure in dcf Jeff Guo
2021-01-12  9:32 ` [dpdk-dev] [dpdk-dev v3 0/3] net/ice: refactor PTYPE parsing Jeff Guo
2021-01-12  9:32   ` [dpdk-dev] [dpdk-dev v3 1/3] net/ice/base: add package PTYPE enable information Jeff Guo
2021-01-12  9:32   ` [dpdk-dev] [dpdk-dev v3 2/3] net/ice/base: add PTYPE value Jeff Guo
2021-01-12  9:32   ` [dpdk-dev] [dpdk-dev v3 3/3] net/ice: refactor PTYPE parsing Jeff Guo
2021-01-13  5:31 ` [dpdk-dev] [dpdk-dev v4 0/2] " Jeff Guo
2021-01-13  5:31   ` [dpdk-dev] [dpdk-dev v4 1/2] net/ice/base: add PTYPE value Jeff Guo
2021-01-13  5:31   ` [dpdk-dev] [dpdk-dev v4 2/2] net/ice: refactor PTYPE parsing Jeff Guo
2021-01-13  6:07   ` [dpdk-dev] [dpdk-dev v4 0/2] " Zhang, Qi Z
2021-01-13 14:05 ` [dpdk-dev] [dpdk-dev v3 0/3] enable UDP ecpri configure in dcf Jeff Guo
2021-01-13 14:05   ` [dpdk-dev] [dpdk-dev v3 1/3] net/ice: add PTYPE mapping for ecpri Jeff Guo
2021-01-13 14:05   ` [dpdk-dev] [dpdk-dev v3 2/3] net/iavf: " Jeff Guo
2021-01-13 14:05   ` [dpdk-dev] [dpdk-dev v3 3/3] net/ice: enable ecpri tunnel port configure in dcf Jeff Guo
2021-01-14  4:18   ` [dpdk-dev] [dpdk-dev v3 0/3] enable UDP ecpri " Zhang, Qi Z
2021-01-18  9:28     ` Ferruh Yigit
2021-01-15  2:42 ` [dpdk-dev] [dpdk-dev v3 0/2] add new UDP tunnel port for ecpri Jeff Guo
2021-01-15  2:42   ` [dpdk-dev] [dpdk-dev v3 1/2] ethdev: add new tunnel type " Jeff Guo
2021-01-15  2:42   ` [dpdk-dev] [dpdk-dev v3 2/2] app/testpmd: add new UDP tunnel port " Jeff Guo
2021-01-15  4:35 ` [dpdk-dev] [dpdk-dev v4 0/2] add new UDP tunnel port configure for eCPRI Jeff Guo
2021-01-15  4:35   ` [dpdk-dev] [dpdk-dev v4 1/2] ethdev: add new tunnel type " Jeff Guo
2021-01-15  4:35   ` [dpdk-dev] [dpdk-dev v4 2/2] app/testpmd: add new UDP tunnel port " Jeff Guo
2021-01-15  5:15 ` [dpdk-dev] [dpdk-dev v5 0/2] add new UDP tunnel port configure " Jeff Guo
2021-01-15  5:15   ` [dpdk-dev] [dpdk-dev v5 1/2] ethdev: add new tunnel type " Jeff Guo
2021-01-15 12:23     ` Ferruh Yigit
2021-01-18  2:40       ` Guo, Jia
2021-01-15  5:15   ` [dpdk-dev] [dpdk-dev v5 2/2] app/testpmd: add new UDP tunnel port " Jeff Guo
2021-01-15 12:24     ` Ferruh Yigit
2021-01-15 12:28   ` [dpdk-dev] [dpdk-dev v5 0/2] add new UDP tunnel port configure " Ferruh Yigit
2021-01-19  3:59 ` [dpdk-dev] [dpdk-dev v4] net/ice: enable eCPRI tunnel port configure in dcf Jeff Guo
2021-01-19  4:15 ` Jeff Guo
2021-01-19  4:19 ` [dpdk-dev] [dpdk-dev v5] " Jeff Guo
2021-01-20 10:14   ` Zhang, Qi Z
2021-01-20 10:19     ` Thomas Monjalon
2021-01-20 10:23       ` Zhang, Qi Z
2021-01-20 10:30         ` Thomas Monjalon
2021-01-20 10:36           ` Zhang, Qi Z
2021-01-20 10:39             ` Thomas Monjalon

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=c619e74fd486481c874dc39104817091@intel.com \
    --to=qi.z.zhang@intel.com \
    --cc=ajit.khaparde@broadcom.com \
    --cc=andrew.rybchenko@oktetlabs.ru \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=getelson@nvidia.com \
    --cc=haiyue.wang@intel.com \
    --cc=jerinj@marvell.com \
    --cc=jia.guo@intel.com \
    --cc=jingjing.wu@intel.com \
    --cc=maxime.coquelin@redhat.com \
    --cc=orika@nvidia.com \
    --cc=qiming.yang@intel.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