linux-kernel - Re: [PATCH net-next v2 03/11] net: dsa: microchip: split ksz

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]

Message-ID: <20201116100724.gbdb4cv2unv4n6zd@skbuf>
Date:   Mon, 16 Nov 2020 12:07:24 +0200
From:   Vladimir Oltean <olteanv@...il.com>
To:     Christian Eggers <ceggers@...i.de>
Cc:     Jakub Kicinski <kuba@...nel.org>, Andrew Lunn <andrew@...n.ch>,
        Richard Cochran <richardcochran@...il.com>,
        Rob Herring <robh+dt@...nel.org>,
        Vivien Didelot <vivien.didelot@...il.com>,
        "David S . Miller" <davem@...emloft.net>,
        Kurt Kanzenbach <kurt.kanzenbach@...utronix.de>,
        George McCollister <george.mccollister@...il.com>,
        Marek Vasut <marex@...x.de>,
        Helmut Grohne <helmut.grohne@...enta.de>,
        Paul Barker <pbarker@...sulko.com>,
        Codrin Ciubotariu <codrin.ciubotariu@...rochip.com>,
        Tristram Ha <Tristram.Ha@...rochip.com>,
        Woojung Huh <woojung.huh@...rochip.com>,
        Microchip Linux Driver Support <UNGLinuxDriver@...rochip.com>,
        netdev@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v2 03/11] net: dsa: microchip: split ksz_common.h

On Mon, Nov 16, 2020 at 10:21:14AM +0100, Christian Eggers wrote:
> On Friday, 13 November 2020, 17:56:34 CET, Christian Eggers wrote:
> > On Friday, 13 November 2020, 00:02:54 CET, Vladimir Oltean wrote:
> > > On Thu, Nov 12, 2020 at 04:35:29PM +0100, Christian Eggers wrote:
> > > > Parts of ksz_common.h (struct ksz_device) will be required in
> > > > net/dsa/tag_ksz.c soon. So move the relevant parts into a new header
> > > > file.
> > > >
> > > > Signed-off-by: Christian Eggers <ceggers@...i.de>
> > > > ---
> > >
> > > I had to skip ahead to see what you're going to use struct ksz_port and
> > >
> > > struct ksz_device for. It looks like you need:
> > >     struct ksz_port::tstamp_rx_latency_ns
> > >     struct ksz_device::ptp_clock_lock
> > >     struct ksz_device::ptp_clock_time
> > >
> > > Not more.
> I have tried to put these members into separate structs:
>
> include/linux/dsa/ksz_common.h:
> struct ksz_port_ptp_shared {
>         u16 tstamp_rx_latency_ns;   /* rx delay from wire to tstamp unit */
> };
>
> struct ksz_device_ptp_shared {
>         spinlock_t ptp_clock_lock; /* for ptp_clock_time */
>         /* approximated current time, read once per second from hardware */
>         struct timespec64 ptp_clock_time;
> };
>
> drivers/net/dsa/microchip/ksz_common.h:
> ...
> #include <linux/dsa/ksz_common.h>
> ...
> struct ksz_port {
> ...
> #if IS_ENABLED(CONFIG_NET_DSA_MICROCHIP_KSZ9477_PTP)
>         struct ksz_port_ptp_shared ptp_shared;  /* shared with tag_ksz.c */
>         u16 tstamp_tx_latency_ns;       /* tx delay from tstamp unit to wire */
>         struct hwtstamp_config tstamp_config;
>         struct sk_buff *tstamp_tx_xdelay_skb;
>         unsigned long tstamp_state;
> #endif
> };
> ...
> struct ksz_device {
> ...
> #if IS_ENABLED(CONFIG_NET_DSA_MICROCHIP_KSZ9477_PTP)
>         struct ptp_clock *ptp_clock;
>         struct ptp_clock_info ptp_caps;
>         struct mutex ptp_mutex;
>         struct ksz_device_ptp_shared ptp_shared;   /* shared with tag_ksz.c */
> #endif
> };
>
> The problem with such technique is, that I still need to dereference
> struct ksz_device in tag_ksz.c:
>
> static void ksz9477_rcv_timestamp(struct sk_buff *skb, u8 *tag,
>                                   struct net_device *dev, unsigned int port)
> {
> ...
>         struct dsa_switch *ds = dev->dsa_ptr->ds;
>         struct ksz_device *ksz = ds->priv;
>         struct ksz_port *prt = &ksz->ports[port];
> ...
> }
>
> As struct dsa_switch::priv is already occupied by the pointer to
> struct ksz_device, I see no way accessing the ptp specific device/port
> information in tag_ksz.c.

There is a dp->priv that you could use to hold a reference to your
PTP-specific substructure (struct ksz_port_ptp_shared) of
struct ksz_port.

Then, in that PTP-specific per-port substructure, you could hold another
pointer to a common struct ksz_device_ptp_shared.