netdev - RE: [PATCH net v2] net: renesas: rswitch: Fix timestamp feature after all descriptors are used

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:
 <TYBPR01MB5341D374AC1DE6CFEE237647D850A@TYBPR01MB5341.jpnprd01.prod.outlook.com>
Date: Thu, 8 Jun 2023 00:20:33 +0000
From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>
To: Hao Lan <lanhao@...wei.com>, "s.shtylyov@....ru" <s.shtylyov@....ru>,
	"davem@...emloft.net" <davem@...emloft.net>, "edumazet@...gle.com"
	<edumazet@...gle.com>, "kuba@...nel.org" <kuba@...nel.org>,
	"pabeni@...hat.com" <pabeni@...hat.com>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-renesas-soc@...r.kernel.org" <linux-renesas-soc@...r.kernel.org>,
	Phong Hoang <phong.hoang.wz@...esas.com>
Subject: RE: [PATCH net v2] net: renesas: rswitch: Fix timestamp feature after
 all descriptors are used

Hi Hao,

> From: Hao Lan, Sent: Wednesday, June 7, 2023 8:28 PM
> 
> On 2023/6/7 15:01, Yoshihiro Shimoda wrote:
> > diff --git a/drivers/net/ethernet/renesas/rswitch.c b/drivers/net/ethernet/renesas/rswitch.c
> > index aace87139cea..049adbf5a642 100644
> > --- a/drivers/net/ethernet/renesas/rswitch.c
> > +++ b/drivers/net/ethernet/renesas/rswitch.c
> > @@ -420,7 +420,7 @@ static int rswitch_gwca_queue_format(struct net_device *ndev,
> >  }
> >
> >  static void rswitch_gwca_ts_queue_fill(struct rswitch_private *priv,
> > -				       int start_index, int num)
> > +				       int start_index, int num, bool last)
> >  {
> >  	struct rswitch_gwca_queue *gq = &priv->gwca.ts_queue;
> >  	struct rswitch_ts_desc *desc;
> > @@ -431,6 +431,12 @@ static void rswitch_gwca_ts_queue_fill(struct rswitch_private *priv,
> >  		desc = &gq->ts_ring[index];
> >  		desc->desc.die_dt = DT_FEMPTY_ND | DIE;
> >  	}
> > +
> > +	if (last) {
> > +		desc = &gq->ts_ring[gq->ring_size];
> > +		rswitch_desc_set_dptr(&desc->desc, gq->ring_dma);
> > +		desc->desc.die_dt = DT_LINKFIX;
> > +	}
> >  }
> >
> Hello Yoshihiro Shimoda，
> 
> Does your function set the last descriptor to hardware on initialization, but not at other times?

The last descriptor initialization is needed at the first time. So, after the initialization,
the last setting will not run anymore.

> I think rswitch_gwca_ts_queue_fill should be implemented in a separate function,
> not use the 'last' distinguish the last descriptor.

I got it. I'll modify this patch on v3.

> But if it should be setting every cycle, I think rswitch_gwca_queue_ext_ts_fill should
> check if the descriptor is the last in the queue and set the LINKFIX flag.

Thank you for the comment. The last descriptor should not be setting every cycle.
To implement the code for consistency, I think that I should add rswitch_tsdesc_init()
rswitch_gwca_queue_format() like rswitch_txdmac_init() and rswitch_gwca_queue_format()

Best regards,
Yoshihiro Shimoda

> >  static int rswitch_gwca_queue_ext_ts_fill(struct net_device *ndev,
> > @@ -941,7 +947,7 @@ static void rswitch_ts(struct rswitch_private *priv)
> >  	}
> >
> >  	num = rswitch_get_num_cur_queues(gq);
> > -	rswitch_gwca_ts_queue_fill(priv, gq->dirty, num);
> > +	rswitch_gwca_ts_queue_fill(priv, gq->dirty, num, false);
> >  	gq->dirty = rswitch_next_queue_index(gq, false, num);
> >  }
> >
> > @@ -1780,7 +1786,7 @@ static int rswitch_init(struct rswitch_private *priv)
> >  	if (err < 0)
> >  		goto err_ts_queue_alloc;
> >
> > -	rswitch_gwca_ts_queue_fill(priv, 0, TS_RING_SIZE);
> > +	rswitch_gwca_ts_queue_fill(priv, 0, TS_RING_SIZE, true);
> >  	INIT_LIST_HEAD(&priv->gwca.ts_info_list);
> >
> >  	for (i = 0; i < RSWITCH_NUM_PORTS; i++) {
> >