On 5 Apr 2022, at 17:13, Paolo Valerio wrote:
> Aaron Conole <[email protected]> writes:
>
>> Eelco Chaudron <[email protected]> writes:
>>
>>> On 4 Apr 2022, at 23:54, Paolo Valerio wrote:
>>>
>>>> Aaron Conole <[email protected]> writes:
>>>>
>>>>> Paolo Valerio <[email protected]> writes:
>>>>>
>>>>>> The next header contained in the last extension header of the IPv6
>>>>>> later frags still contain the information of the upper layer protocol
>>>>>> number.
>>>>>>
>>>>>> Similarly to what OvS does for IPv4, allow L4 matches for later IPv6
>>>>>> frags as well by processing later frags and storing the nw_proto
>>>>>> information.
>>>>>>
>>>>>> Signed-off-by: Paolo Valerio <[email protected]>
>>>>>> ---
>>>>>
>>>>> Digging in, I could never find a reason why ipv6 was treated differently
>>>>> in this manner, but I guess there could be cases where the header layout
>>>>> could cause problems (see the top of the while() block). I don't know
>>>>> that it's a practical concern (other values are probably less common),
>>>>> but it was the only thing I could come up with for why the match might
>>>>> behave this way. I haven't done enough mailing list archeology to have
>>>>> a good idea.
>>>>
>>>> Guess this may probably be due to (rfc8200 section-4.5):
>>>>
>>>> "The subsequent fragment packets are composed of:
>>>>
>>>> [...]
>>>>
>>>> (2) A Fragment header containing:
>>>>
>>>> The Next Header value that identifies the first header
>>>> after the Per-Fragment headers of the original packet.
>>>>
>>>> [...]"
>>>>
>>>> where the "first header" in the original packet could be another
>>>> extension header or an upper-layer header, while the Fragment header is
>>>> the last EH for later fragments (according to section 4.5).
>>
>> Thanks for digging in and finding this.
>>
>>>> If I'm not missing something, it makes sense to drop the patch.
>>>> It's probably a good thing if we document the different behavior between
>>>> IPv4 and IPv6 for later frags, though.
>>>
>>> Alternatively, we could check if the next header is a non-extension header
>>> and if so, mark it as such.
>>>
>>> The only problem might be the case where packets do have extension
>>> headers after the fragmentation header, and they might be dropped
>>> (handled differently).
>>
>> I think it might be best to keep the system as-is. If we add this
>> "feature," users will still need to do matches like:
>>
>> table=X,ip6,ct_state=-trk actions=ct(...)
>>
>> They can try to optimize the flow table looks by doing individual
>> selections, but it will be confusing to flow writers (and take an extra
>> bit of time to debug) when they get missed packets. Even if we try to
>> make things convenient, there can be edge cases we miss - and today
>> systems like OVN already accommodate the nuance of v4 vs v6.
>>
>
> I agree. Although the proposal is reasonable and covers the majority of
> the cases, it's probably better to stick with the current behavior.
>
>> I'm in favor of a documentation patch instead that spells it out for
>> flow writers to clearly understand. That way we don't have a custom
>> state machine to try and work around this limitation.
>>
>
> If we all agree, I'd proceed with that.
Sound good to me, see below ;)
>>> Anyhow, whatever we decide, we should document it in both the documentation
>>> and code.
>>> //Eelco
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
