| Message ID | 20221025222005.4074737-1-jan@3e8.eu |
|---|---|
| Headers | show |
| Series | realtek: fix L2 entry setup and learning on CPU port | expand |
Hi Jan, On Wed, 2022-10-26 at 00:20 +0200, Jan Hoffmann wrote: > This is a follow-up to the patch "realtek: don't set L2LEARNING flag in > rtl83xx TX header". An undesired effect of that patch is flooding of > some packets destined for the switch CPU port, which is addressed by > this additional patch series. > > This patch series switches to assisted learning for the CPU port on all > devices, and also fixes some existing issues with setup of unicast L2 > entries. > > Together with the kernel 5.15 pull request, entries for local/bridge > addresses are added to the switch. I am still not sure why that doesn't > work with the patches in the current kernel. However, the pull request > for the kernel update seems to be in a good shape, so I don't think it > is worth it to investigate that any further. > > Tested on RTL838x (HPE 1920-8G) and RTL839x (HPE 1920-48G). > > Changes in v2: > - don't explicitly specify struct name as parameter to sizeof > - make calculation of SALRN shift offset clearer > - define SALRN values, mask and shift offset in header > > Jan Hoffmann (2): > realtek: set up L2 table entries properly > realtek: use assisted learning on CPU port > > .../files-5.10/drivers/net/dsa/rtl83xx/dsa.c | 45 ++++++++++++++----- > .../drivers/net/dsa/rtl83xx/rtl838x.h | 6 +++ > 2 files changed, 41 insertions(+), 10 deletions(-) Thanks for the respin! Both patches have been applied on master. Best, Sander
On 26.10.22 at 10:20, Sander Vanheule wrote: > Hi Jan, > > On Wed, 2022-10-26 at 00:20 +0200, Jan Hoffmann wrote: >> This is a follow-up to the patch "realtek: don't set L2LEARNING flag in >> rtl83xx TX header". An undesired effect of that patch is flooding of >> some packets destined for the switch CPU port, which is addressed by >> this additional patch series. >> >> This patch series switches to assisted learning for the CPU port on all >> devices, and also fixes some existing issues with setup of unicast L2 >> entries. >> >> Together with the kernel 5.15 pull request, entries for local/bridge >> addresses are added to the switch. I am still not sure why that doesn't >> work with the patches in the current kernel. However, the pull request >> for the kernel update seems to be in a good shape, so I don't think it >> is worth it to investigate that any further. >> >> Tested on RTL838x (HPE 1920-8G) and RTL839x (HPE 1920-48G). >> >> Changes in v2: >> - don't explicitly specify struct name as parameter to sizeof >> - make calculation of SALRN shift offset clearer >> - define SALRN values, mask and shift offset in header >> >> Jan Hoffmann (2): >> realtek: set up L2 table entries properly >> realtek: use assisted learning on CPU port >> >> .../files-5.10/drivers/net/dsa/rtl83xx/dsa.c | 45 ++++++++++++++----- >> .../drivers/net/dsa/rtl83xx/rtl838x.h | 6 +++ >> 2 files changed, 41 insertions(+), 10 deletions(-) > > Thanks for the respin! Both patches have been applied on master. Thank you for applying the patches. My intention for this series was for it to be applied in addition to the previous patch "realtek: don't set L2LEARNING flag in rtl83xx TX header", but that patch isn't applied. I guess my wording in the cover letter was somewhat ambiguous here, and maybe I should have just included the change in this series. However, in terms of actual functionality it shouldn't matter anyways, as the L2LEARNING flag in the TX header doesn't have any effect now that learning is disabled using the SALRN register. In the end, it just means that the explanation of the FDB corruption issue isn't in the Git history, and the commit message of eb456aedfe24 ("realtek: use assisted learning on CPU port") is now slightly confusing, as it references the change to the TX header which doesn't actually exist. Best, Jan
On Wed, 2022-10-26 at 23:40 +0200, Jan Hoffmann wrote: > > > On 26.10.22 at 10:20, Sander Vanheule wrote: > > Hi Jan, > > > > On Wed, 2022-10-26 at 00:20 +0200, Jan Hoffmann wrote: > > > This is a follow-up to the patch "realtek: don't set L2LEARNING flag in > > > rtl83xx TX header". An undesired effect of that patch is flooding of > > > some packets destined for the switch CPU port, which is addressed by > > > this additional patch series. > > > > > > This patch series switches to assisted learning for the CPU port on all > > > devices, and also fixes some existing issues with setup of unicast L2 > > > entries. > > > > > > Together with the kernel 5.15 pull request, entries for local/bridge > > > addresses are added to the switch. I am still not sure why that doesn't > > > work with the patches in the current kernel. However, the pull request > > > for the kernel update seems to be in a good shape, so I don't think it > > > is worth it to investigate that any further. > > > > > > Tested on RTL838x (HPE 1920-8G) and RTL839x (HPE 1920-48G). > > > > > > Changes in v2: > > > - don't explicitly specify struct name as parameter to sizeof > > > - make calculation of SALRN shift offset clearer > > > - define SALRN values, mask and shift offset in header > > > > > > Jan Hoffmann (2): > > > realtek: set up L2 table entries properly > > > realtek: use assisted learning on CPU port > > > > > > .../files-5.10/drivers/net/dsa/rtl83xx/dsa.c | 45 ++++++++++++++----- > > > .../drivers/net/dsa/rtl83xx/rtl838x.h | 6 +++ > > > 2 files changed, 41 insertions(+), 10 deletions(-) > > > > Thanks for the respin! Both patches have been applied on master. > > Thank you for applying the patches. > > My intention for this series was for it to be applied in addition to the > previous patch "realtek: don't set L2LEARNING flag in rtl83xx TX > header", but that patch isn't applied. I guess my wording in the cover > letter was somewhat ambiguous here, and maybe I should have just > included the change in this series. The L2LEARNING patch did include a lot of extra info, which I wasn't sure I understood yet. I suppose I should've asked you to clarify sooner. > > However, in terms of actual functionality it shouldn't matter anyways, > as the L2LEARNING flag in the TX header doesn't have any effect now that > learning is disabled using the SALRN register. In the end, it just means > that the explanation of the FDB corruption issue isn't in the Git > history, and the commit message of eb456aedfe24 ("realtek: use assisted > learning on CPU port") is now slightly confusing, as it references the > change to the TX header which doesn't actually exist. If the learning bit in the TX headers is now redundant, and would cause trouble when exposed again, I think that patch is still applicable. I'll mark the L2LEARNING patch as "deferred". May I ask you to submit a new series with that patch, and the int/u32 fixup? If you have fixed any other related (address learning) issues in the mean time, you're free to add those too. Best, Sander