Re: [PATCH net-next v2 0/8] Add support for two classes of VCAP rules

Michael Walle
2023-01-06T14:19:26Z

Hi Steen,

>> > > Wouldn't it make more sense, to fix the regression via net (and
>> > > a Fixes: tag) and then make that stuff work without tc? Maybe
>> > > the fix is just reverting the commits.
>> >
>> > I have discussed this again with Horatiu and I have the following
>> > suggestion of
>> > how to proceed:
>> >
>> > 1) Create a small LAN966x specific patch for net (see below for the two
>> > possible
>> >    variants).
>> >
>> > 2) Continue with a net-next V3 without any 'Fixes' tags on top of the
>> > patch in
>> >    (1) when it becomes available in net-next.
>>
>> Sounds good.
>>
>> [coming back to this after writing the response below, so see there
>> for more context]
>> When do the patches from net become available in net-next? Only after
>> a
>> merge window? If so, depending on the solution for (1) you'd have two
>> "in-between" kernel versions (v6.2 and v6.3).
>
> According to our own experience the changes in net are usually merged
> into net-
> next the following Thursday: so not too much delay, before we can
> continue.

TIL :)

>> > The LAN966x patch for net (with a Fixes tag) could contain either:
>> >
>> > a) No check on enabled lookup
>> >
>> >    Removal of the check for enabled lookups:
>> >
>> >    -  if (!ANA_VCAP_S2_CFG_ENA_GET(val))
>> >    -          return -ENOENT;
>> >
>> >    This will remove the error that you have seen, but  will still
>> > require a
>> >    matchall rule to enable the PTP rules.  This is compatible with the
>> > TC
>> >    framework.
>> >
>> > b) Always enable lookups
>> >
>> >    Enable the lookups at startup.
>> >    Remove the lookup enable check as above.
>> >
>> >    This will make the PTP rules (and any other rules) work even without
>> > the
>> >    matchall rule to enable them.  It its not ideal, but solves the
>> > problem that
>> >    you have been experiencing without the 'TC magic'
>> >
>> >    The V3 in net-next will provide the full solution.
>> >
>> > I expect that you might prefer the b) version.
>>
>> I *assume* linuxptp would have worked in my case (no bridge interface)
>> before Horatiu patches. As mentioned before, I haven't really tested
>> it.
>> Does that mean with a) the error is gone and linuxptp is working as
>> before? If so, I'm also fine with a).
>
> Yes this is the result: So I also suggest to go for solution a).
>
> This will still allow LinuxPTP to work (without the error that you have
> seen),
> but the bridged interface PTP support must be enabled with a TC
> matchall rule.
>
>>
>> Honestly, now that there is a good solution in future kernels, I
>> don't care toooo much about that one particular kernel. Other
>> users might disagree though ;)
>>
>> I just want to point out that right now you have some kind of
>> in-between kernel with 6.2:
>>
>>   <=6.1 linuxptp working (but not on bridged ports)
>>   6.2   linuxptp working only with tc magic
>>   6.3   linuxptp working
>
> So with the LAN966x patch the second line would change to:
>
> 6.2 linuxptp working. PTP on bridged interfaces: needs TC matchall
> rule
>
>>
>> Therefore, I've raised the question if it's also viable to just
>> revert the former changes for 6.2. The you'd have a clean
>> transition.
>>
>> -michael
>
> TLDR Summary:
>
> 1) LAN966x patch for net to ensure PTP is working without errors
> 2) A V3 net-next VCAP series with the improvements for
> enabled/disable/permanent
> rules (both LAN966x and Sparx5)
>
> I will move forward with this.

Sounds perfect, thanks!

-michael