Re: [RFC V1 1/1] net: extend VXLAN header to support more extensions

[Ferruh Yigit <ferruh.yigit@amd.com>]

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.

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?


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?