DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ilya Maximets <i.maximets@ovn.org>
To: Thomas Monjalon <thomas@monjalon.net>
Cc: i.maximets@ovn.org, dev@dpdk.org, stable@dpdk.org,
	Ajit Khaparde <ajit.khaparde@broadcom.com>,
	Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>,
	Hemant Agrawal <hemant.agrawal@nxp.com>,
	Haiyue Wang <haiyue.wang@intel.com>,
	John Daley <johndale@cisco.com>,
	Guoyang Zhou <zhouguoyang@huawei.com>,
	"Min Hu (Connor)" <humin29@huawei.com>,
	Beilei Xing <beilei.xing@intel.com>,
	Jingjing Wu <jingjing.wu@intel.com>,
	Qi Zhang <qi.z.zhang@intel.com>, Rosen Xu <rosen.xu@intel.com>,
	Matan Azrad <matan@nvidia.com>,
	Viacheslav Ovsiienko <viacheslavo@nvidia.com>,
	Liron Himi <lironh@marvell.com>,
	Jiawen Wu <jiawenwu@trustnetic.com>, Ori Kam <orika@nvidia.com>
Subject: Re: [PATCH] doc: fix support table for ETH and VLAN flow items
Date: Fri, 7 Oct 2022 13:50:01 +0200	[thread overview]
Message-ID: <c42f7afd-349e-1cd3-0ad8-d2f6272918ce@ovn.org> (raw)
In-Reply-To: <2843961.e9J7NaK4W3@thomas>

On 9/12/22 12:09, Thomas Monjalon wrote:
> 16/03/2022 13:01, Ilya Maximets:
>> 'has_vlan' attribute is only supported by sfc, mlx5 and cnxk.
>> Other drivers doesn't support it.  Most of them (like i40e) just
>> ignore it silently.  Some drivers (like mlx4) never had a full
>> support of the eth item even before introduction of 'has_vlan'
>> (mlx4 allows to match on the destination MAC only).
>>
>> Same for the 'has_more_vlan' flag of the vlan item.
>>
>> Changing the support level to 'partial' for all such drivers.
>> This doesn't solve the issue, but at least marks the problematic
>> drivers.
> 
> You changed "eth" and "vlan" from "Y" to "P".
> The field "has_vlan" is part of "rte_flow_item_eth",
> and "has_more_vlan" is part of "rte_flow_item_vlan",
> so I agree we need to change both items to "partial support".
> It looks to be a good change, just needs to more explicit,
> adding this kind of explanation about the fields.

I can add this to the commit message.  Should I also add
some note alongside the table in that documentation page?

> 
> We missed this patch in 22.07, let's have some progress quickly.

It was a busy month + PTO, sorry I didn't get to that patch faster.

> 
>> Some details are available in:
>>   https://bugs.dpdk.org/show_bug.cgi?id=958
> 
> About rte_flow_item_eth.{src,dst}, I don't find a deprecation notice
> about it in the history of the file doc/guides/rel_notes/deprecation.rst

It took some time to find the deprecation notice, but I did find it
in the end.  It's in the doc/guides/rel_notes/deprecation.rst, and
it says:

* ethdev: The flow API matching pattern structures, ``struct rte_flow_item_*``,
  should start with relevant protocol header.
  Some matching pattern structures implements this by duplicating protocol header
  fields in the struct. To clarify the intention and to be sure protocol header
  is intact, will replace those fields with relevant protocol header struct.
  In v21.02 both individual protocol header fields and the protocol header struct
  will be added as union, target is switch usage to the protocol header by time.
  In v21.11 LTS, protocol header fields will be cleaned and only protocol header
  struct will remain.

> This is what we have in the code:
>     union {
>         struct {
>             /*
>              * These fields are retained for compatibility.
>              * Please switch to the new header field below.
>              */
>             struct rte_ether_addr dst; /**< Destination MAC. */
>             struct rte_ether_addr src; /**< Source MAC. */
>             rte_be16_t type; /**< EtherType or TPID. */
>         };
>         struct rte_ether_hdr hdr;
>     };
> 
> Do you think we should remove the old fields now?

From the OVS perspective, we do not care much.  OVS is still using
old fields, but it's fairly simple change that we can do while moving
to a new version of DPDK.

From the perspective of the API clarity, it's probably better to clean
up these structures.

Best regards, Ilya Maximets.

> 
>> Fixes: 09315fc83861 ("ethdev: add VLAN attributes to ethernet and VLAN items")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
> 
> 
> 


  parent reply	other threads:[~2022-10-07 11:50 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-16 12:01 Ilya Maximets
2022-04-20 17:51 ` Ferruh Yigit
2022-04-26  8:55   ` Asaf Penso
2022-04-26 10:47     ` Ferruh Yigit
2022-04-27 11:02       ` Asaf Penso
2022-09-12 10:09 ` Thomas Monjalon
2022-09-12 11:08   ` Ori Kam
2022-10-07 11:50   ` Ilya Maximets [this message]
2022-10-07 11:55     ` Ilya Maximets
2022-10-12 15:30       ` Thomas Monjalon
2022-10-13 11:01         ` Ilya Maximets
2022-10-13 12:18           ` 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=c42f7afd-349e-1cd3-0ad8-d2f6272918ce@ovn.org \
    --to=i.maximets@ovn.org \
    --cc=ajit.khaparde@broadcom.com \
    --cc=beilei.xing@intel.com \
    --cc=dev@dpdk.org \
    --cc=haiyue.wang@intel.com \
    --cc=hemant.agrawal@nxp.com \
    --cc=humin29@huawei.com \
    --cc=jiawenwu@trustnetic.com \
    --cc=jingjing.wu@intel.com \
    --cc=johndale@cisco.com \
    --cc=lironh@marvell.com \
    --cc=matan@nvidia.com \
    --cc=orika@nvidia.com \
    --cc=qi.z.zhang@intel.com \
    --cc=rahul.lakkireddy@chelsio.com \
    --cc=rosen.xu@intel.com \
    --cc=stable@dpdk.org \
    --cc=thomas@monjalon.net \
    --cc=viacheslavo@nvidia.com \
    --cc=zhouguoyang@huawei.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).