netdev - Re: [PATCH v2 2/7] net: dsa: b53: Move struct b53

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 9 Nov 2021 20:49:03 +0200
From:   Vladimir Oltean <olteanv@...il.com>
To:     Florian Fainelli <f.fainelli@...il.com>
Cc:     Martin Kaistra <martin.kaistra@...utronix.de>,
        Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Richard Cochran <richardcochran@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        John Stultz <john.stultz@...aro.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Stephen Boyd <sboyd@...nel.org>,
        Russell King <linux@...linux.org.uk>,
        Marc Kleine-Budde <mkl@...gutronix.de>,
        linux-kernel@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCH v2 2/7] net: dsa: b53: Move struct b53_device to
 include/linux/dsa/b53.h

On Tue, Nov 09, 2021 at 10:20:25AM -0800, Florian Fainelli wrote:
> On 11/9/21 10:15 AM, Vladimir Oltean wrote:
> > On Tue, 9 Nov 2021 at 20:11, Florian Fainelli <f.fainelli@...il.com> wrote:
> >>
> >> On 11/9/21 10:05 AM, Florian Fainelli wrote:
> >>> On 11/9/21 1:50 AM, Martin Kaistra wrote:
> >>>> In order to access the b53 structs from net/dsa/tag_brcm.c move the
> >>>> definitions from drivers/net/dsa/b53/b53_priv.h to the new file
> >>>> include/linux/dsa/b53.h.
> >>>>
> >>>> Signed-off-by: Martin Kaistra <martin.kaistra@...utronix.de>
> >>>> ---
> >>>>  drivers/net/dsa/b53/b53_priv.h |  90 +----------------------------
> >>>>  include/linux/dsa/b53.h        | 100 +++++++++++++++++++++++++++++++++
> >>>>  2 files changed, 101 insertions(+), 89 deletions(-)
> >>>>  create mode 100644 include/linux/dsa/b53.h
> >>>
> >>> All you really access is the b53_port_hwtstamp structure within the
> >>> tagger, so please make it the only structure exposed to net/dsa/tag_brcm.c.
> >>
> >> You do access b53_dev in the TX part, still, I would like to find a more
> >> elegant solution to exposing everything here, can you create a
> >> b53_timecounter_cyc2time() function that is exported to modules but does
> >> not require exposing the b53_device to net/dsa/tag_brcm.c?
> >> --
> >> Florian
> > 
> > Switch drivers can't export symbols to tagging protocol drivers, remember?
> > https://lore.kernel.org/netdev/20210908220834.d7gmtnwrorhharna@skbuf/
> 
> I do now :) How about a function pointer in dsa_switch_ops that driver
> can hook onto?

IMHO, the honest answer to this is to admit that it's not quite okay to
timestamp a single packet at a time and simply not timestamp any packet
that might be concurrent with that, no warning or questions asked. We
should queue that second packet if it cannot be marked for timestamping
right away.

Which brings me to my second point, there used to be a generic deferred
xmit mechanism in DSA that also happened to solve this problem because
yes, there was a function pointer in dsa_switch_ops for it.
But you and Richard didn't quite like it, so I removed it.
https://patchwork.ozlabs.org/cover/1215617/