netdev - Re: [net-next,v9] net: ethernet: rtsn: Add support for Renesas Ethernet-TSN

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: Thu, 20 Jun 2024 13:40:46 +0200
From: Niklas Söderlund <niklas.soderlund+renesas@...natech.se>
To: Ratheesh Kannoth <rkannoth@...vell.com>
Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Andrew Lunn <andrew@...n.ch>, netdev@...r.kernel.org,
	linux-renesas-soc@...r.kernel.org
Subject: Re: [net-next,v9] net: ethernet: rtsn: Add support for Renesas
 Ethernet-TSN

Hi Ratheesh,

Thanks for your feedback.

On 2024-06-18 17:27:16 +0530, Ratheesh Kannoth wrote:
> On 2024-06-18 at 14:38:24, Niklas Söderlund (niklas.soderlund+renesas@...natech.se) wrote:
> > Add initial support for Renesas Ethernet-TSN End-station device of R-Car
> > V4H. The Ethernet End-station can connect to an Ethernet network using a
> > 10 Mbps, 100 Mbps, or 1 Gbps full-duplex link via MII/GMII/RMII/RGMII.
> > Depending on the connected PHY.
> >
> > The driver supports Rx checksum and offload and hardware timestamps.
> >
> > While full power management and suspend/resume is not yet supported the
> > driver enables runtime PM in order to enable the module clock. While
> > explicit clock management using clk_enable() would suffice for the
> > supported SoC, the module could be reused on SoCs where the module is
> > part of a power domain.
> >
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@...natech.se>
> > ---
> > +static int rtsn_tx_free(struct net_device *ndev, bool free_txed_only)
> > +{
> > +	struct rtsn_private *priv = netdev_priv(ndev);
> > +	struct rtsn_ext_desc *desc;
> > +	struct sk_buff *skb;
> > +	int free_num = 0;
> > +	int entry, size;
> > +
> > +	for (; priv->cur_tx - priv->dirty_tx > 0; priv->dirty_tx++) {
> > +		entry = priv->dirty_tx % priv->num_tx_ring;
> > +		desc = &priv->tx_ring[entry];
> > +		if (free_txed_only && (desc->die_dt & DT_MASK) != DT_FEMPTY)
> > +			break;
> > +
> > +		dma_rmb();
> > +		size = le16_to_cpu(desc->info_ds) & TX_DS;
> > +		skb = priv->tx_skb[entry];
> > +		if (skb) {
> > +			if (skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP) {
> > +				struct skb_shared_hwtstamps shhwtstamps;
> > +				struct timespec64 ts;
> > +
> > +				rtsn_get_timestamp(priv, &ts);
> > +				memset(&shhwtstamps, 0, sizeof(shhwtstamps));
> > +				shhwtstamps.hwtstamp = timespec64_to_ktime(ts);
> > +				skb_tstamp_tx(skb, &shhwtstamps);
> > +			}
> > +			dma_unmap_single(ndev->dev.parent,
> > +					 le32_to_cpu(desc->dptr),
> > +					 size, DMA_TO_DEVICE);
> > +			dev_kfree_skb_any(priv->tx_skb[entry]);
> > +			free_num++;
> > +		}
> > +		desc->die_dt = DT_EEMPTY;
> > +		priv->stats.tx_packets++;
> if (!skb) case also, incrementing the tx_packets. Is this correct ?

It could be moved inside the skb check, but it will have no real effect 
as the for loop is broken on the first descriptor that is not empty. But 
seems like a v10 will be needed anyhow I will move it.

-- 
Kind Regards,
Niklas Söderlund