forked from mirrors/linux
		
	net: renesas: rswitch: rework ts tags management
The existing linked list based implementation of how ts tags are
assigned and managed is unsafe against concurrency and corner cases:
- element addition in tx processing can race against element removal
  in ts queue completion,
- element removal in ts queue completion can race against element
  removal in device close,
- if a large number of frames gets added to tx queue without ts queue
  completions in between, elements with duplicate tag values can get
  added.
Use a different implementation, based on per-port used tags bitmaps and
saved skb arrays.
Safety for addition in tx processing vs removal in ts completion is
provided by:
    tag = find_first_zero_bit(...);
    smp_mb();
    <write rdev->ts_skb[tag]>
    set_bit(...);
  vs
    <read rdev->ts_skb[tag]>
    smp_mb();
    clear_bit(...);
Safety for removal in ts completion vs removal in device close is
provided by using atomic read-and-clear for rdev->ts_skb[tag]:
    ts_skb = xchg(&rdev->ts_skb[tag], NULL);
    if (ts_skb)
        <handle it>
Fixes: 33f5d733b5 ("net: renesas: rswitch: Improve TX timestamp accuracy")
Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
Link: https://patch.msgid.link/20241212062558.436455-1-nikita.yoush@cogentembedded.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
			
			
This commit is contained in:
		
							parent
							
								
									cb85f2b897
								
							
						
					
					
						commit
						922b4b955a
					
				
					 2 changed files with 39 additions and 42 deletions
				
			
		|  | @ -547,7 +547,6 @@ static int rswitch_gwca_ts_queue_alloc(struct rswitch_private *priv) | |||
| 	desc = &gq->ts_ring[gq->ring_size]; | ||||
| 	desc->desc.die_dt = DT_LINKFIX; | ||||
| 	rswitch_desc_set_dptr(&desc->desc, gq->ring_dma); | ||||
| 	INIT_LIST_HEAD(&priv->gwca.ts_info_list); | ||||
| 
 | ||||
| 	return 0; | ||||
| } | ||||
|  | @ -1003,9 +1002,10 @@ static int rswitch_gwca_request_irqs(struct rswitch_private *priv) | |||
| static void rswitch_ts(struct rswitch_private *priv) | ||||
| { | ||||
| 	struct rswitch_gwca_queue *gq = &priv->gwca.ts_queue; | ||||
| 	struct rswitch_gwca_ts_info *ts_info, *ts_info2; | ||||
| 	struct skb_shared_hwtstamps shhwtstamps; | ||||
| 	struct rswitch_ts_desc *desc; | ||||
| 	struct rswitch_device *rdev; | ||||
| 	struct sk_buff *ts_skb; | ||||
| 	struct timespec64 ts; | ||||
| 	unsigned int num; | ||||
| 	u32 tag, port; | ||||
|  | @ -1015,23 +1015,28 @@ static void rswitch_ts(struct rswitch_private *priv) | |||
| 		dma_rmb(); | ||||
| 
 | ||||
| 		port = TS_DESC_DPN(__le32_to_cpu(desc->desc.dptrl)); | ||||
| 		tag = TS_DESC_TSUN(__le32_to_cpu(desc->desc.dptrl)); | ||||
| 		if (unlikely(port >= RSWITCH_NUM_PORTS)) | ||||
| 			goto next; | ||||
| 		rdev = priv->rdev[port]; | ||||
| 
 | ||||
| 		list_for_each_entry_safe(ts_info, ts_info2, &priv->gwca.ts_info_list, list) { | ||||
| 			if (!(ts_info->port == port && ts_info->tag == tag)) | ||||
| 				continue; | ||||
| 		tag = TS_DESC_TSUN(__le32_to_cpu(desc->desc.dptrl)); | ||||
| 		if (unlikely(tag >= TS_TAGS_PER_PORT)) | ||||
| 			goto next; | ||||
| 		ts_skb = xchg(&rdev->ts_skb[tag], NULL); | ||||
| 		smp_mb(); /* order rdev->ts_skb[] read before bitmap update */ | ||||
| 		clear_bit(tag, rdev->ts_skb_used); | ||||
| 
 | ||||
| 		if (unlikely(!ts_skb)) | ||||
| 			goto next; | ||||
| 
 | ||||
| 		memset(&shhwtstamps, 0, sizeof(shhwtstamps)); | ||||
| 		ts.tv_sec = __le32_to_cpu(desc->ts_sec); | ||||
| 		ts.tv_nsec = __le32_to_cpu(desc->ts_nsec & cpu_to_le32(0x3fffffff)); | ||||
| 		shhwtstamps.hwtstamp = timespec64_to_ktime(ts); | ||||
| 			skb_tstamp_tx(ts_info->skb, &shhwtstamps); | ||||
| 			dev_consume_skb_irq(ts_info->skb); | ||||
| 			list_del(&ts_info->list); | ||||
| 			kfree(ts_info); | ||||
| 			break; | ||||
| 		} | ||||
| 		skb_tstamp_tx(ts_skb, &shhwtstamps); | ||||
| 		dev_consume_skb_irq(ts_skb); | ||||
| 
 | ||||
| next: | ||||
| 		gq->cur = rswitch_next_queue_index(gq, true, 1); | ||||
| 		desc = &gq->ts_ring[gq->cur]; | ||||
| 	} | ||||
|  | @ -1576,8 +1581,9 @@ static int rswitch_open(struct net_device *ndev) | |||
| static int rswitch_stop(struct net_device *ndev) | ||||
| { | ||||
| 	struct rswitch_device *rdev = netdev_priv(ndev); | ||||
| 	struct rswitch_gwca_ts_info *ts_info, *ts_info2; | ||||
| 	struct sk_buff *ts_skb; | ||||
| 	unsigned long flags; | ||||
| 	unsigned int tag; | ||||
| 
 | ||||
| 	netif_tx_stop_all_queues(ndev); | ||||
| 
 | ||||
|  | @ -1594,12 +1600,13 @@ static int rswitch_stop(struct net_device *ndev) | |||
| 	if (bitmap_empty(rdev->priv->opened_ports, RSWITCH_NUM_PORTS)) | ||||
| 		iowrite32(GWCA_TS_IRQ_BIT, rdev->priv->addr + GWTSDID); | ||||
| 
 | ||||
| 	list_for_each_entry_safe(ts_info, ts_info2, &rdev->priv->gwca.ts_info_list, list) { | ||||
| 		if (ts_info->port != rdev->port) | ||||
| 			continue; | ||||
| 		dev_kfree_skb_irq(ts_info->skb); | ||||
| 		list_del(&ts_info->list); | ||||
| 		kfree(ts_info); | ||||
| 	for (tag = find_first_bit(rdev->ts_skb_used, TS_TAGS_PER_PORT); | ||||
| 	     tag < TS_TAGS_PER_PORT; | ||||
| 	     tag = find_next_bit(rdev->ts_skb_used, TS_TAGS_PER_PORT, tag + 1)) { | ||||
| 		ts_skb = xchg(&rdev->ts_skb[tag], NULL); | ||||
| 		clear_bit(tag, rdev->ts_skb_used); | ||||
| 		if (ts_skb) | ||||
| 			dev_kfree_skb(ts_skb); | ||||
| 	} | ||||
| 
 | ||||
| 	return 0; | ||||
|  | @ -1612,20 +1619,17 @@ static bool rswitch_ext_desc_set_info1(struct rswitch_device *rdev, | |||
| 	desc->info1 = cpu_to_le64(INFO1_DV(BIT(rdev->etha->index)) | | ||||
| 				  INFO1_IPV(GWCA_IPV_NUM) | INFO1_FMT); | ||||
| 	if (skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP) { | ||||
| 		struct rswitch_gwca_ts_info *ts_info; | ||||
| 		unsigned int tag; | ||||
| 
 | ||||
| 		ts_info = kzalloc(sizeof(*ts_info), GFP_ATOMIC); | ||||
| 		if (!ts_info) | ||||
| 		tag = find_first_zero_bit(rdev->ts_skb_used, TS_TAGS_PER_PORT); | ||||
| 		if (tag == TS_TAGS_PER_PORT) | ||||
| 			return false; | ||||
| 		smp_mb(); /* order bitmap read before rdev->ts_skb[] write */ | ||||
| 		rdev->ts_skb[tag] = skb_get(skb); | ||||
| 		set_bit(tag, rdev->ts_skb_used); | ||||
| 
 | ||||
| 		skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS; | ||||
| 		rdev->ts_tag++; | ||||
| 		desc->info1 |= cpu_to_le64(INFO1_TSUN(rdev->ts_tag) | INFO1_TXC); | ||||
| 
 | ||||
| 		ts_info->skb = skb_get(skb); | ||||
| 		ts_info->port = rdev->port; | ||||
| 		ts_info->tag = rdev->ts_tag; | ||||
| 		list_add_tail(&ts_info->list, &rdev->priv->gwca.ts_info_list); | ||||
| 		desc->info1 |= cpu_to_le64(INFO1_TSUN(tag) | INFO1_TXC); | ||||
| 
 | ||||
| 		skb_tx_timestamp(skb); | ||||
| 	} | ||||
|  |  | |||
|  | @ -972,14 +972,6 @@ struct rswitch_gwca_queue { | |||
| 	}; | ||||
| }; | ||||
| 
 | ||||
| struct rswitch_gwca_ts_info { | ||||
| 	struct sk_buff *skb; | ||||
| 	struct list_head list; | ||||
| 
 | ||||
| 	int port; | ||||
| 	u8 tag; | ||||
| }; | ||||
| 
 | ||||
| #define RSWITCH_NUM_IRQ_REGS	(RSWITCH_MAX_NUM_QUEUES / BITS_PER_TYPE(u32)) | ||||
| struct rswitch_gwca { | ||||
| 	unsigned int index; | ||||
|  | @ -989,7 +981,6 @@ struct rswitch_gwca { | |||
| 	struct rswitch_gwca_queue *queues; | ||||
| 	int num_queues; | ||||
| 	struct rswitch_gwca_queue ts_queue; | ||||
| 	struct list_head ts_info_list; | ||||
| 	DECLARE_BITMAP(used, RSWITCH_MAX_NUM_QUEUES); | ||||
| 	u32 tx_irq_bits[RSWITCH_NUM_IRQ_REGS]; | ||||
| 	u32 rx_irq_bits[RSWITCH_NUM_IRQ_REGS]; | ||||
|  | @ -997,6 +988,7 @@ struct rswitch_gwca { | |||
| }; | ||||
| 
 | ||||
| #define NUM_QUEUES_PER_NDEV	2 | ||||
| #define TS_TAGS_PER_PORT	256 | ||||
| struct rswitch_device { | ||||
| 	struct rswitch_private *priv; | ||||
| 	struct net_device *ndev; | ||||
|  | @ -1004,7 +996,8 @@ struct rswitch_device { | |||
| 	void __iomem *addr; | ||||
| 	struct rswitch_gwca_queue *tx_queue; | ||||
| 	struct rswitch_gwca_queue *rx_queue; | ||||
| 	u8 ts_tag; | ||||
| 	struct sk_buff *ts_skb[TS_TAGS_PER_PORT]; | ||||
| 	DECLARE_BITMAP(ts_skb_used, TS_TAGS_PER_PORT); | ||||
| 	bool disabled; | ||||
| 
 | ||||
| 	int port; | ||||
|  |  | |||
		Loading…
	
		Reference in a new issue
	
	 Nikita Yushchenko
						Nikita Yushchenko