DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ferruh Yigit <ferruh.yigit@amd.com>
To: Thomas Monjalon <thomas@monjalon.net>,
	Gavin Li <gavinl@nvidia.com>,
	orika@nvidia.com, andrew.rybchenko@oktetlabs.ru
Cc: dev@dpdk.org, jiaweiw@nvidia.com
Subject: Re: [RFC V1 1/1] net: extend VXLAN header to support more extensions
Date: Fri, 9 Feb 2024 15:58:55 +0000	[thread overview]
Message-ID: <50486980-5a35-48d5-adc6-db29fcf5786a@amd.com> (raw)
In-Reply-To: <27994279.gRfpFWEtPU@thomas>

On 2/9/2024 3:32 PM, Thomas Monjalon wrote:
> 09/02/2024 15:58, Ferruh Yigit:
>> On 2/9/2024 1:44 PM, Thomas Monjalon wrote:
>>> 09/02/2024 13:11, Ferruh Yigit:
>>>> On 2/9/2024 10:12 AM, Thomas Monjalon wrote:
>>>>> 09/02/2024 00:54, Ferruh Yigit:
>>>>>> On 1/30/2024 11:25 AM, Gavin Li wrote:
>>>>>>> Currently, DPDK supports VXLAN and VXLAN-GPE with similar header
>>>>>>> structures and we are working on adding support for VXLAN-GBP which is
>>>>>>> another extension to VXLAN. More extension of VXLAN may be added in the
>>>>>>> future.
>>>>>>>
>>>>>>> VXLAN and VXLAN-GBP use the same UDP port(4789) while VXLAN-GPE uses a
>>>>>>> different one, 4790. The three protocols have the same header length and
>>>>>>> overall similar header structure as below.
>>>>>>>     0                   1                   2                   3
>>>>>>>     0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
>>>>>>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>>>    |R|R|R|R|I|R|R|R|            Reserved                           |
>>>>>>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>>>    |                VXLAN Network Identifier (VNI) |   Reserved    |
>>>>>>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>>>
>>>>>>>                            Figure 1: VXLAN Header
>>>>>>>
>>>>>>>     0                   1                   2                   3
>>>>>>>     0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
>>>>>>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>>>    |R|R|Ver|I|P|B|O|       Reserved                |Next Protocol  |
>>>>>>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>>>    |                VXLAN Network Identifier (VNI) |   Reserved    |
>>>>>>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>>>
>>>>>>>                          Figure 2: VXLAN-GPE Header
>>>>>>>
>>>>>>>     0                   1                   2                   3
>>>>>>>     0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
>>>>>>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>>>    |G|R|R|R|I|R|R|R|R|D|R|R|A|R|R|R|        Group Policy ID        |
>>>>>>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>>>    |          VXLAN Network Identifier (VNI)       |   Reserved    |
>>>>>>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>>>>>
>>>>>>>                           Figure 3: VXLAN-GBP Extension
>>>>>>>
>>>>>>> Both VXLAN-GPE and VXLAN-GBP extended VXLAN by redefining its reserved
>>>>>>> bits, which means the packets can be processed with same pattern and most
>>>>>>> of the code can be reused. Instead of adding more new items by
>>>>>>> copying/pasting code for the VXLAN extensions in the future, it’s better
>>>>>>> to use existing VXLAN infrastructure and add support code in it.
>>>>>>>
>>>>>>
>>>>>> Hi Gavin,
>>>>>>
>>>>>> The motivation is to prevent code duplication, and the code mentioned is
>>>>>>  the driver code, right?
>>>>>
>>>>> The motivation is mainly to provide a unified and more explicit API.
>>>>>
>>>>
>>>> From user perspective, I think existing approach is more explicit,
>>>> because it sets VXLAN or VXLAN_GPE flow types.
>>>>
>>>> I am trying to understand the benefit, how unifying flow type in the API
>>>> helps to the user?
>>>>
>>>>>> Overall OK to unify "struct rte_vxlan_hdr" although it makes the struct
>>>>>> a little complex, perhaps we can consider extraction some nested structs
>>>>>> as named struct, no strong opinion.
>>>>>>
>>>>>>
>>>>>> But not sure about removing the flow item types for VXLAN-GPE, or not
>>>>>> adding for VXLAN-GBP.
>>>>>>
>>>>>> Think about a case user adding a rule, which has a item type as VXLAN
>>>>>> and in the protocol header some bits are set, lets say first word, last
>>>>>> byte is set, how driver will know if to take it as GPE "next protocol"
>>>>>> or "group policy id".
>>>>>
>>>>> The driver may decide depending on the UDP port and some distinguishing flags.
>>>>> If you want to match on GBP, you should includes the GBP flag in your pattern,
>>>>> no need to use a separate item.
>>>>>
>>>>
>>>> Why not be more explicit?
>>>> It helps to driver to know more about the pattern to be able to create
>>>> proper flow rule, if there is an obvious way for driver to differentiate
>>>> these protocol extensions, and flow item type is redundant, I can
>>>> understand the proposal, but otherwise I think better to keep flow items
>>>> for extensions.
>>>
>>> In any case we need the simple VXLAN item.
>>> If we have GPE and GBP specialized items,
>>> what means a match on the simple VXLAN?
>>> Does it include packets with other extensions or exclude them?
>>> Matching the bits in the protocol make such decision explicit.
>>>
>>>> When a rule is set in HW, HW may not care about the protocol, as long as
>>>> bits in the rule match with the packet, HW can apply the action.
>>>> But for driver to be able to set the rule properly, it needs more
>>>> explicit information.
>>>
>>> Yes information is in the pattern to match.
>>>
>>>> Lets assume driver API get a pattern with 'RTE_FLOW_ITEM_TYPE_VXLAN'
>>>> type and "struct rte_flow_item_vxlan", at this point driver doesn't know
>>>> if it is VXLAN or any of the extensions.
>>>
>>> Yes it knows because of the matched bits in the pattern.
>>> If the rule specify a match on GBP flag = 1, it is GBP only.
>>> If the rule specify a match on GBP flag = 0, it excludes GBP.
>>> If the rule does not mask GBP flag, it includes GBP.
>>>
>>
>>
>> OK, VXLAN-GBP protocol has a GBP flag that gives a way to differentiate
>> the extension, so flow item for it becomes redundant and we can get rid
>> of it.
> 
> Yes I think so.
> 
>> Is it same for the other extensions?
>> If we use VXLAN flow item and by setting specific field in pattern can
>> we differentiate VXLAN and any other extension?
>> Or in some cases other information, like UDP port, needs to be taken
>> into account to differentiate protocol/extension?
> 
> For VXLAN-GPE, differentiation is on UDP port.
> Remember we have an API to fill some UDP ports:
> rte_eth_dev_udp_tunnel_port_add with RTE_ETH_TUNNEL_TYPE_VXLAN_GPE
> 
> The UDP port value/mask may be part of the flow rule pattern.
> 

So one option is use vxlan extension specific flow item, which is
current case.

Other option is use generic vxlan flow item, and detect the extension
using other fields, which is this proposal.

Both works, but specially if driver needs other protocol information,
like UDP port, to detect the vxlan protocol extension, why not just keep
continue with existing explicit flow item.


OK for merging vxlan struct in net library.
But I am failing to see benefit to change the flow item and structs.
No strong opinion, it can be good to get more comments.


> 
>> I found a spec for VXLAN-GBP, but it shows as sub-header for VXLAN-GPE,
>> different than what this RFC describes:
>> https://datatracker.ietf.org/doc/html/draft-lemon-vxlan-gpe-gbp
>>
>> Can you please share link for VXLAN-GBP Extension spec?
> 
> I will let Gavin explain here, I'm not an expert.
> 
> 


  reply	other threads:[~2024-02-09 15:59 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-30 11:25 [RFC V1 0/1] " Gavin Li
2024-01-30 11:25 ` [RFC V1 1/1] " Gavin Li
2024-02-06 22:51   ` Thomas Monjalon
2024-02-07  4:49     ` Ajit Khaparde
2024-02-08 23:54   ` Ferruh Yigit
2024-02-09 10:12     ` Thomas Monjalon
2024-02-09 12:11       ` Ferruh Yigit
2024-02-09 13:44         ` Thomas Monjalon
2024-02-09 14:58           ` Ferruh Yigit
2024-02-09 15:32             ` Thomas Monjalon
2024-02-09 15:58               ` Ferruh Yigit [this message]
2024-02-19  3:16               ` Gavin Li
2024-02-19  3:44               ` Gavin Li
2024-02-19  4:03                 ` Gavin Li

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=50486980-5a35-48d5-adc6-db29fcf5786a@amd.com \
    --to=ferruh.yigit@amd.com \
    --cc=andrew.rybchenko@oktetlabs.ru \
    --cc=dev@dpdk.org \
    --cc=gavinl@nvidia.com \
    --cc=jiaweiw@nvidia.com \
    --cc=orika@nvidia.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
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).