Re: question about eth and vlan item in flow pattern

[Jie Hai <haijie1@huawei.com>]

On 2024/4/25 5:46, Ivan Malov wrote:
> Hi,
> 
> On Wed, 24 Apr 2024, Dariusz Sosnowski wrote:
> 

>>
>> To expand the argument on convenience by providing an example,
>> `has_vlan` allows to construct a following logic:
>>
>> 1) flow create 0 group X ingress pattern eth has_vlan is 1 / end 
>> actions jump group Y / end
>> 2) flow create 0 group X ingress pattern eth has_vlan is 0 / end 
>> actions jump group Z / end
>>
>> 3) flow create 0 group Y ingress pattern eth / vlan vid is N / end 
>> actions /* actions for tagged traffic */
>> 4) flow create 0 group Z ingress pattern eth / end actions /* actions 
>> for non-tagged traffic */
>>
>> Traffic can be split between different groups - one group for tagged 
>> and another for non-tagged packets.
>> `has_vlan` gives more flexibility in the API.
>>
>>>>
>>>> There are drivers seeing `vlan` an one vlan layer and match 
>>>> 'eth/vlan/end'
>>>> with single-tagged packets, e.g.bnxt, hns3, etc.
>>>> I think this meaning makes it easier to distinguish the functions of 
>>>> the two.
>>>>
>>>> The `has_more_vlan is 1` is defined and used by some drivers, like 
>>>> mlx5.
>>>> While the `has_more_vlan is 0` is not defined.
>>> What do you mean by saying "not defined"? Not defined where?
>>> Having "has_more_vlan is 0" is perfectly legit, isn't it?
>>>
>>>> Here you give the meaning as `having no vlan after this vlan`.
>>>> Then, What should the follow rule mean? single-tagged or double tagged?
>>>>
>>>> - pattern eth / vlan tpid is 0x8100 has_more_vlan is 0 / end
>>> Item VLAN here is supposed to describe the header _immediately_ 
>>> following
>>> the ETH one, isn't it? If so, the rule means single-tagged. Doesn't it?
>>
>> I also think this rule means single-tagged, but on the other hand I 
>> think it's also ill-defined.
> 
> Thank you for noticing this. My bad. Indeed, this rule is just incorrect
> and shall be rejected by the driver -- that is because "tpid is 0x8100"
> means that the header following the outermost VLAN tag is yet another
> VLAN tag, whereas "has_more_vlan is 0" wants to match on the opposite,
> namely on the absence of the inner tag. So, in this rule, one match
> criterion contradicts another one. Parser should see through this.
> 
> Thank you.

Hi, all ,

I have some questions
There are some other contradicted conditions:
1. eth has_vlan is 0 / vlan / end XXX
2. eth / vlan has_more_vlan is 0/ vlan / end  XXX
I think rule 1 and rule2 are both contradicted and should be  rejected 
by drivers.

3. eth  has_vlan is 1  / end XXX
4. eth  / vlan has_more_vlan is 1  / end XXX
5. eth has_vlan is 1 / vlan has_more_vlan is 1  / end XXX
6. eth has_vlan is 1 / vlan has_more_vlan is 0  / end XXX

Rule 3 should match tagged(>=1) and rule 4 should match
multi-tagged(>=2) is definite.

Since 'has_vlan is 1' is redundant with 'vlan'.
'has_vlan is 1' and 'vlan has_more_vlan is X',
which one should be ignored, and which one should be effective.
If 'has_vlan is 1' should be ignored， rule 5 is same as rule 4,
and rule 6 should match single-tagged.
If 'vlan has_more_vlan is X' should be ignored， rule 5 and rule 6
is the same and it seems literally unconscionable.

What do you think about?
Thanks you.

> 
>> tpid field in VLAN item defines the inner packet type,
>> so this pattern should match the following packet:
>>
>> dmac=<any>
>> smac=<any>
>> ether_type=0x8100  // match single-tagged packet
>> pcp=<any>
>> vid=<any>
>> ether_type=0x8100  // from tpid VLAN item
>>
>> Which is an incorrect packet.
>>
>>> Thank you.
>>>
>>>>
>>>>> With regard to the question about VLAN ID match, "pattern eth / vlan
>>>>> vid is 10", as well as "pattern eth has_vlan is 1 / vlan vid is 10",
>>>>> should probably match only on the outermost tag ID, -- that is, not
>>>>> on the inner one and not on both.
>>>> I agree with it.
>>>>> That is because the "vid is 10" criterion relates to the specific
>>>>> header, as per the match item it sits in. If the user wants to match
>>>>> on the inner VLAN ID (say, 11), they should clarify specify it as 
>>>>> follows:
>>>>>
>>>>> - pattern eth / vlan vid is 10 / vlan vid is 11 / end
>>>>> - pattern eth / vlan / vlan vid is 11 / end
>>>>>
>>>>
>>>>> Hopefully, the rest of DPDK vendor community can correct me in case I
>>>>> got that wrong. Should you have more questions, please feel free to 
>>>>> ask
>>> such.
>>>>>
>>>>> Thank you.
>>>>>
>>>>> On Wed, 10 Apr 2024, Jie Hai wrote:
>>>>>
>>>>>> Hi, all,
>>>>>>
>>>>>> I have some questions about the sub-options for ``VLAN`` and ``ETH``
>>> item.
>>>>>>
>>>>>> According to the documentation, ``has_vlan`` is sub-option of
>>>>>> ``ETH`` item and it means that the pattern contains at least one 
>>>>>> vlan.
>>>>>> The ``VLAN`` item is used to match tagged packets and have some
>>>>>> sub-options such as ``vid``, ``tci``, etc.
>>>>>>
>>>>>> If we combine them, what should the effect be?
>>>>>> For instance,
>>>>>>
>>>>>> rule-0: flow create 0 ingress pattern  eth  has_vlan is 1 / vlan  /
>>>>>> end actions queue index 2 / end
>>>>>> rule-1: flow create 0 ingress pattern  eth  has_vlan is 1 / vlan vid
>>>>>> is 10 / end actions queue index 2 / end
>>>>>>
>>>>>> For rule-0, should it match single-tagged packets only or
>>>>>> multi-tagged only or both?
>>>>>> That is to say, which one will take effect, `has_vlan is 1`  or
>>>>>> `vlan` or both?
>>>>>>
>>>>>> For rule-2, which packets should it match, with inner VLAN id 10, or
>>>>>> outer VLAN id 10, or both 10?
>>>>>>
>>>>>> The hns3 driver supports only the exact matching of VLAN numer.
>>>>>> And it is planned to adapt ``has_vlan`` and ``has_more_vlan`` to the
>>>>>> meaning of one VLAN for hns3 driver. Therefore, if the preceding
>>>>>> combinations are supported, we need to confirm the exact meanings.
>>>>>>
>>>>>> So, what are your views on the above question?
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>> .
>>>>
>>