History

net: prevent torn reads in netdev_tc_txq

netdev_set_tc_queue() (and related helpers/drivers such as netdev_bind_sb_channel_queue(), netdev_reset_tc(), and netdev_unbind_sb_channel()) perform separate 16-bit writes to dev->tc_to_txq[tc].count and dev->tc_to_txq[tc].offset. Furthermore, memset() in netdev_reset_tc() and netdev_unbind_sb_channel() provides no guarantee of performing full 32-bit word stores. Concurrent lockless readers (e.g. skb_tx_hash(), netdev_txq_to_tc(), ixgbe_select_queue(), taprio, mqprio, FPE drivers) can observe torn values where offset and count belong to inconsistent configurations. Redefine struct netdev_tc_txq to embed count and offset inside a union with a u32 combined field, allowing atomic manipulation via READ_ONCE() and WRITE_ONCE(). Update all lockless readers and writers across the kernel to use READ_ONCE() and WRITE_ONCE() on the combined field.

Eric Dumazet committed 3 months ago

Commit 21ef2d01 parent e6a5d5711 files · +94 −43Browse files

Unified · Split

11 files
  1. drivers/net/ethernet/intel/igc/igc_tsn.c +4 −2
  2. drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +4 −3
  3. drivers/net/ethernet/mellanox/mlx5/core/en_main.c +1 −1
  4. drivers/net/ethernet/sfc/falcon/tx.c +6 −2
  5. drivers/net/ethernet/sfc/siena/tx.c +6 −2
  6. drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.c +10 −4
  7. include/linux/netdevice.h +7 −2
  8. net/core/dev.c +33 −13
  9. net/sched/sch_mqprio.c +3 −1
  10. net/sched/sch_mqprio_lib.c +5 −2
  11. net/sched/sch_taprio.c +15 −11
drivers/net/ethernet/intel/igc/igc_tsn.c+4−2
@@ -183,13 +183,15 @@ static u32 igc_fpe_map_preempt_tc_to_queue(const struct igc_adapter *adapter, · 182 unmodified lines
183183 u32 i, queue = 0;
184184
185185 for (i = 0; i < dev->num_tc; i++) {
186+ struct netdev_tc_txq res;
186187 u32 offset, count;
187188
188189 if (!(preemptible_tcs & BIT(i)))
189190 continue;
190191
191− offset = dev->tc_to_txq[i].offset;
192− count = dev->tc_to_txq[i].count;
192+ res.combined = READ_ONCE(dev->tc_to_txq[i].combined);
193+ offset = res.offset;
194+ count = res.count;
193195 queue |= GENMASK(offset + count - 1, offset);
194196 }
195197
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c+4−3
@@ -9273,10 +9273,11 @@ static u16 ixgbe_select_queue(struct net_device *dev, struct sk_buff *skb, · 9272 unmodified lines
92739273 if (sb_dev) {
92749274 u8 tc = netdev_get_prio_tc_map(dev, skb->priority);
92759275 struct net_device *vdev = sb_dev;
9276+ struct netdev_tc_txq res;
92769277
9277− txq = vdev->tc_to_txq[tc].offset;
9278− txq += reciprocal_scale(skb_get_hash(skb),
9279− vdev->tc_to_txq[tc].count);
9278+ res.combined = READ_ONCE(vdev->tc_to_txq[tc].combined);
9279+ txq = res.offset;
9280+ txq += reciprocal_scale(skb_get_hash(skb), res.count);
92809281
92819282 return txq;
92829283 }
drivers/net/ethernet/mellanox/mlx5/core/en_main.c+1−1
@@ -3247,7 +3247,7 @@ static int mlx5e_update_tc_and_tx_queues(struct mlx5e_priv *priv) · 3246 unmodified lines
32473247 old_num_txqs = netdev->real_num_tx_queues;
32483248 old_ntc = netdev->num_tc ? : 1;
32493249 for (i = 0; i < ARRAY_SIZE(old_tc_to_txq); i++)
3250− old_tc_to_txq[i] = netdev->tc_to_txq[i];
3250+ old_tc_to_txq[i].combined = READ_ONCE(netdev->tc_to_txq[i].combined);
32513251
32523252 nch = priv->channels.params.num_channels;
32533253 ntc = priv->channels.params.mqprio.num_tc;
drivers/net/ethernet/sfc/falcon/tx.c+6−2
@@ -439,8 +439,12 @@ int ef4_setup_tc(struct net_device *net_dev, enum tc_setup_type type, · 438 unmodified lines
439439 return 0;
440440
441441 for (tc = 0; tc < num_tc; tc++) {
442− net_dev->tc_to_txq[tc].offset = tc * efx->n_tx_channels;
443− net_dev->tc_to_txq[tc].count = efx->n_tx_channels;
442+ struct netdev_tc_txq res = {
443+ .offset = tc * efx->n_tx_channels,
444+ .count = efx->n_tx_channels,
445+ };
446+
447+ WRITE_ONCE(net_dev->tc_to_txq[tc].combined, res.combined);
444448 }
445449
446450 if (num_tc > net_dev->num_tc) {
drivers/net/ethernet/sfc/siena/tx.c+6−2
@@ -380,8 +380,12 @@ int efx_siena_setup_tc(struct net_device *net_dev, enum tc_setup_type type, · 379 unmodified lines
380380 return 0;
381381
382382 for (tc = 0; tc < num_tc; tc++) {
383− net_dev->tc_to_txq[tc].offset = tc * efx->n_tx_channels;
384− net_dev->tc_to_txq[tc].count = efx->n_tx_channels;
383+ struct netdev_tc_txq res = {
384+ .offset = tc * efx->n_tx_channels,
385+ .count = efx->n_tx_channels,
386+ };
387+
388+ WRITE_ONCE(net_dev->tc_to_txq[tc].combined, res.combined);
385389 }
386390
387391 net_dev->num_tc = num_tc;
drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.c+10−4
@@ -217,8 +217,11 @@ int dwmac5_fpe_map_preemption_class(struct net_device *ndev, · 216 unmodified lines
217217 * and is direct one-to-one mapping."
218218 */
219219 for (u32 tc = 0; tc < num_tc; tc++) {
220− count = ndev->tc_to_txq[tc].count;
221− offset = ndev->tc_to_txq[tc].offset;
220+ struct netdev_tc_txq res;
221+
222+ res.combined = READ_ONCE(ndev->tc_to_txq[tc].combined);
223+ count = res.count;
224+ offset = res.offset;
222225
223226 if (pclass & BIT(tc))
224227 preemptible_txqs |= GENMASK(offset + count - 1, offset);
@@ -275,8 +278,11 @@ int dwxgmac3_fpe_map_preemption_class(struct net_device *ndev, · 50 unmodified lines
275278 * any of the scheduling algorithms."
276279 */
277280 for (u32 tc = 0; tc < num_tc; tc++) {
278− count = ndev->tc_to_txq[tc].count;
279− offset = ndev->tc_to_txq[tc].offset;
281+ struct netdev_tc_txq res;
282+
283+ res.combined = READ_ONCE(ndev->tc_to_txq[tc].combined);
284+ count = res.count;
285+ offset = res.offset;
280286
281287 if (pclass & BIT(tc))
282288 preemptible_txqs |= GENMASK(offset + count - 1, offset);
include/linux/netdevice.h+7−2
@@ -832,8 +832,13 @@ struct xps_dev_maps { · 831 unmodified lines
832832 #define TC_BITMASK 15
833833 /* HW offloaded queuing disciplines txq count and offset maps */
834834 struct netdev_tc_txq {
835− u16 count;
836− u16 offset;
835+ union {
836+ struct {
837+ u16 count;
838+ u16 offset;
839+ };
840+ u32 combined;
841+ };
837842 };
838843
839844 #if defined(CONFIG_FCOE) || defined(CONFIG_FCOE_MODULE)
net/core/dev.c+33−13
@@ -2652,11 +2652,13 @@ EXPORT_SYMBOL_GPL(dev_queue_xmit_nit); · 2651 unmodified lines
26522652 */
26532653 static void netif_setup_tc(struct net_device *dev, unsigned int txq)
26542654 {
2655+ struct netdev_tc_txq res;
26552656 int i;
2656− struct netdev_tc_txq *tc = &dev->tc_to_txq[0];
2657+
2658+ res.combined = READ_ONCE(dev->tc_to_txq[0].combined);
26572659
26582660 /* If TC0 is invalidated disable TC mapping */
2659− if (tc->offset + tc->count > txq) {
2661+ if (res.offset + res.count > txq) {
26602662 netdev_warn(dev, "Number of in use tx queues changed invalidating tc mappings. Priority traffic classification disabled!\n");
26612663 dev->num_tc = 0;
26622664 return;
@@ -2666,8 +2668,8 @@ static void netif_setup_tc(struct net_device *dev, unsigned int txq) · 3 unmodified lines
26662668 for (i = 1; i < TC_BITMASK + 1; i++) {
26672669 int q = netdev_get_prio_tc_map(dev, i);
26682670
2669− tc = &dev->tc_to_txq[q];
2670− if (tc->offset + tc->count > txq) {
2671+ res.combined = READ_ONCE(dev->tc_to_txq[q].combined);
2672+ if (res.offset + res.count > txq) {
26712673 netdev_warn(dev, "Number of in use tx queues changed. Priority %i to tc mapping %i is no longer valid. Setting map to 0\n",
26722674 i, q);
26732675 netdev_set_prio_tc_map(dev, i, 0);
@@ -2683,7 +2685,10 @@ int netdev_txq_to_tc(struct net_device *dev, unsigned int txq) · 9 unmodified lines
26832685
26842686 /* walk through the TCs and see if it falls into any of them */
26852687 for (i = 0; i < TC_MAX_QUEUE; i++, tc++) {
2686− if ((txq - tc->offset) < tc->count)
2688+ struct netdev_tc_txq res;
2689+
2690+ res.combined = READ_ONCE(tc->combined);
2691+ if ((txq - res.offset) < res.count)
26872692 return i;
26882693 }
26892694
@@ -3103,6 +3108,8 @@ static void netdev_unbind_all_sb_channels(struct net_device *dev) · 413 unmodified lines
31033108
31043109 void netdev_reset_tc(struct net_device *dev)
31053110 {
3111+ int i;
3112+
31063113 #ifdef CONFIG_XPS
31073114 netif_reset_xps_queues_gt(dev, 0);
31083115 #endif
@@ -3110,21 +3117,26 @@ void netdev_reset_tc(struct net_device *dev) · 1 unmodified line
31103117
31113118 /* Reset TC configuration of device */
31123119 dev->num_tc = 0;
3113− memset(dev->tc_to_txq, 0, sizeof(dev->tc_to_txq));
3120+ for (i = 0; i < TC_MAX_QUEUE; i++)
3121+ WRITE_ONCE(dev->tc_to_txq[i].combined, 0);
31143122 memset(dev->prio_tc_map, 0, sizeof(dev->prio_tc_map));
31153123 }
31163124 EXPORT_SYMBOL(netdev_reset_tc);
31173125
31183126 int netdev_set_tc_queue(struct net_device *dev, u8 tc, u16 count, u16 offset)
31193127 {
3128+ struct netdev_tc_txq res = {
3129+ .count = count,
3130+ .offset = offset,
3131+ };
3132+
31203133 if (tc >= dev->num_tc)
31213134 return -EINVAL;
31223135
31233136 #ifdef CONFIG_XPS
31243137 netif_reset_xps_queues(dev, offset, count);
31253138 #endif
3126− dev->tc_to_txq[tc].count = count;
3127− dev->tc_to_txq[tc].offset = offset;
3139+ WRITE_ONCE(dev->tc_to_txq[tc].combined, res.combined);
31283140 return 0;
31293141 }
31303142 EXPORT_SYMBOL(netdev_set_tc_queue);
@@ -3148,11 +3160,13 @@ void netdev_unbind_sb_channel(struct net_device *dev, · 17 unmodified lines
31483160 struct net_device *sb_dev)
31493161 {
31503162 struct netdev_queue *txq = &dev->_tx[dev->num_tx_queues];
3163+ int i;
31513164
31523165 #ifdef CONFIG_XPS
31533166 netif_reset_xps_queues_gt(sb_dev, 0);
31543167 #endif
3155− memset(sb_dev->tc_to_txq, 0, sizeof(sb_dev->tc_to_txq));
3168+ for (i = 0; i < TC_MAX_QUEUE; i++)
3169+ WRITE_ONCE(sb_dev->tc_to_txq[i].combined, 0);
31563170 memset(sb_dev->prio_tc_map, 0, sizeof(sb_dev->prio_tc_map));
31573171
31583172 while (txq-- != &dev->_tx[0]) {
@@ -3175,8 +3189,12 @@ int netdev_bind_sb_channel_queue(struct net_device *dev, · 16 unmodified lines
31753189 return -EINVAL;
31763190
31773191 /* Record the mapping */
3178− sb_dev->tc_to_txq[tc].count = count;
3179− sb_dev->tc_to_txq[tc].offset = offset;
3192+ struct netdev_tc_txq res = {
3193+ .count = count,
3194+ .offset = offset,
3195+ };
3196+
3197+ WRITE_ONCE(sb_dev->tc_to_txq[tc].combined, res.combined);
31803198
31813199 /* Provide a way for Tx queue to find the tc_to_txq map or
31823200 * XPS map for itself.
@@ -3542,9 +3560,11 @@ static u16 skb_tx_hash(const struct net_device *dev, · 359 unmodified lines
35423560
35433561 if (dev->num_tc) {
35443562 u8 tc = netdev_get_prio_tc_map(dev, skb->priority);
3563+ struct netdev_tc_txq res;
35453564
3546− qoffset = sb_dev->tc_to_txq[tc].offset;
3547− qcount = sb_dev->tc_to_txq[tc].count;
3565+ res.combined = READ_ONCE(sb_dev->tc_to_txq[tc].combined);
3566+ qoffset = res.offset;
3567+ qcount = res.count;
35483568 if (unlikely(!qcount)) {
35493569 net_warn_ratelimited("%s: invalid qcount, qoffset %u for tc %u\n",
35503570 sb_dev->name, qoffset, tc);
net/sched/sch_mqprio.c+3−1
@@ -679,12 +679,14 @@ static int mqprio_dump_class_stats(struct Qdisc *sch, unsigned long cl, · 678 unmodified lines
679679 rcu_read_lock();
680680 if (cl >= TC_H_MIN_PRIORITY) {
681681 struct net_device *dev = qdisc_dev(sch);
682− struct netdev_tc_txq tc = dev->tc_to_txq[cl & TC_BITMASK];
682+ struct netdev_tc_txq tc;
683683 struct gnet_stats_queue qstats = {0};
684684 struct gnet_stats_basic_sync bstats;
685685 u32 qlen = 0;
686686 int i;
687687
688+ tc.combined = READ_ONCE(dev->tc_to_txq[cl & TC_BITMASK].combined);
689+
688690 gnet_stats_basic_sync_init(&bstats);
689691
690692 for (i = tc.offset; i < tc.offset + tc.count; i++) {
net/sched/sch_mqprio_lib.c+5−2
@@ -108,8 +108,11 @@ void mqprio_qopt_reconstruct(struct net_device *dev, struct tc_mqprio_qopt *qopt · 107 unmodified lines
108108 memcpy(qopt->prio_tc_map, dev->prio_tc_map, sizeof(qopt->prio_tc_map));
109109
110110 for (tc = 0; tc < num_tc; tc++) {
111− qopt->count[tc] = dev->tc_to_txq[tc].count;
112− qopt->offset[tc] = dev->tc_to_txq[tc].offset;
111+ struct netdev_tc_txq res;
112+
113+ res.combined = READ_ONCE(dev->tc_to_txq[tc].combined);
114+ qopt->count[tc] = res.count;
115+ qopt->offset[tc] = res.offset;
113116 }
114117 }
115118 EXPORT_SYMBOL_GPL(mqprio_qopt_reconstruct);
net/sched/sch_taprio.c+15−11
@@ -762,12 +762,13 @@ skip_peek_checks: · 761 unmodified lines
762762
763763 static void taprio_next_tc_txq(struct net_device *dev, int tc, int *txq)
764764 {
765− int offset = dev->tc_to_txq[tc].offset;
766− int count = dev->tc_to_txq[tc].count;
765+ struct netdev_tc_txq res;
766+
767+ res.combined = READ_ONCE(dev->tc_to_txq[tc].combined);
767768
768769 (*txq)++;
769− if (*txq == offset + count)
770− *txq = offset;
770+ if (*txq == res.offset + res.count)
771+ *txq = res.offset;
771772 }
772773
773774 /* Prioritize higher traffic classes, and select among TXQs belonging to the
@@ -1441,15 +1442,14 @@ static u32 tc_map_to_queue_mask(struct net_device *dev, u32 tc_mask) · 667 unmodified lines
14411442 u32 i, queue_mask = 0;
14421443
14431444 for (i = 0; i < dev->num_tc; i++) {
1444− u32 offset, count;
1445+ struct netdev_tc_txq res;
14451446
14461447 if (!(tc_mask & BIT(i)))
14471448 continue;
14481449
1449− offset = dev->tc_to_txq[i].offset;
1450− count = dev->tc_to_txq[i].count;
1450+ res.combined = READ_ONCE(dev->tc_to_txq[i].combined);
14511451
1452− queue_mask |= GENMASK(offset + count - 1, offset);
1452+ queue_mask |= GENMASK(res.offset + res.count - 1, res.offset);
14531453 }
14541454
14551455 return queue_mask;
@@ -1802,10 +1802,14 @@ static int taprio_mqprio_cmp(const struct net_device *dev, · 346 unmodified lines
18021802 if (!mqprio || mqprio->num_tc != dev->num_tc)
18031803 return -1;
18041804
1805− for (i = 0; i < mqprio->num_tc; i++)
1806− if (dev->tc_to_txq[i].count != mqprio->count[i] ||
1807− dev->tc_to_txq[i].offset != mqprio->offset[i])
1805+ for (i = 0; i < mqprio->num_tc; i++) {
1806+ struct netdev_tc_txq res;
1807+
1808+ res.combined = READ_ONCE(dev->tc_to_txq[i].combined);
1809+ if (res.count != mqprio->count[i] ||
1810+ res.offset != mqprio->offset[i])
18081811 return -1;
1812+ }
18091813
18101814 for (i = 0; i <= TC_BITMASK; i++)
18111815 if (dev->prio_tc_map[i] != mqprio->prio_tc_map[i])

Keyboard shortcuts

General

Show keyboard shortcuts
?
Focus the current search
/

Issues and pull requests

Open filters
f

Issues, pull requests and Inbox

Select next item
j↓
Select previous item
k↑
Open selected item
Enter
Dismiss filters or clear selection
Esc

Repository code

Go to file
t