[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZNfKeCGf0xZTTum7@vergenet.net>
Date: Sat, 12 Aug 2023 20:07:52 +0200
From: Simon Horman <horms@...nel.org>
To: "Radu Pirea (NXP OSS)" <radu-nicolae.pirea@....nxp.com>
Cc: andrew@...n.ch, hkallweit1@...il.com, linux@...linux.org.uk,
davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
pabeni@...hat.com, richardcochran@...il.com, sd@...asysnail.net,
netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC net-next v1 4/5] net: macsec: introduce mdo_insert_tx_tag
On Fri, Aug 11, 2023 at 06:32:48PM +0300, Radu Pirea (NXP OSS) wrote:
> Offloading MACsec in PHYs requires inserting the SecTAG and the ICV in
> the ethernet frame. This operation will increase the frame size with 32
> bytes. If the frames are sent at line rate, the PHY will not have enough
> room to insert the SecTAG and the ICV.
>
> To mitigate this scenario, the PHY offer to use require a specific
> ethertype with some padding bytes present in the ethernet frame. This
> ethertype and its associated bytes will be replaced by the SecTAG and ICV.
>
> Signed-off-by: Radu Pirea (NXP OSS) <radu-nicolae.pirea@....nxp.com>
...
> diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
...
> @@ -4137,6 +4211,11 @@ static int macsec_newlink(struct net *net, struct net_device *dev,
> if (err)
> goto del_dev;
> }
> +
> + dev->needed_headroom -= MACSEC_NEEDED_HEADROOM;
> + dev->needed_headroom += ops->needed_headroom;
> + dev->needed_tailroom -= MACSEC_NEEDED_TAILROOM;
> + dev->needed_tailroom += ops->needed_tailroom;
Hi Radu,
Just above the beginning of this hunk it is assumed that ops may be NULL.
However, here it is dereferenced unconditionally. Is this safe?
Flagged by Smatch.
> }
>
> err = register_macsec_dev(real_dev, dev);
> diff --git a/include/net/macsec.h b/include/net/macsec.h
> index 33dc7f2aa42e..a988249d9608 100644
> --- a/include/net/macsec.h
> +++ b/include/net/macsec.h
> @@ -272,6 +272,7 @@ struct macsec_context {
> struct macsec_rx_sa_stats *rx_sa_stats;
> struct macsec_dev_stats *dev_stats;
> } stats;
> + struct sk_buff *skb;
Not strictly related to this patch,
but it would be nice to update the kernel doc for this
structure so that it's fields are documented.
> };
>
> /**
> @@ -302,6 +303,10 @@ struct macsec_ops {
> int (*mdo_get_tx_sa_stats)(struct macsec_context *ctx);
> int (*mdo_get_rx_sc_stats)(struct macsec_context *ctx);
> int (*mdo_get_rx_sa_stats)(struct macsec_context *ctx);
> + /* Offload tag */
> + int (*mdo_insert_tx_tag)(struct macsec_context *ctx);
> + int needed_headroom;
> + int needed_tailroom;
Ditto.
> };
>
> #if IS_ENABLED(CONFIG_MACSEC)
> --
> 2.34.1
>
>
Powered by blists - more mailing lists