mbox series

[0/8] reduce open coded skb->next access for gso segment walking

Message ID 20200108215909.421487-1-Jason@zx2c4.com
Headers show
Series reduce open coded skb->next access for gso segment walking | expand

Message

Jason A. Donenfeld Jan. 8, 2020, 9:59 p.m. UTC
This patchset introduces the skb_list_walk_safe helper macro, in order
to add some sanity to the myrid ways drivers have of walking through gso
segments. The goal is to reduce future bugs commonly caused by open
coding these sorts of things, and to in the future make it easier to
swap out the underlying list representation.

This first patch series addresses the easy uses of drivers iterating
over the returned list of skb_gso_segments, for drivers that live in
drivers/net/*. There are still other use cases to tackle later for
net/*, and after these low-hanging fruits are taken care of, I imagine
there are more subtle cases of gso segment walking that isn't just a
direct return value from skb_gso_segments, and eventually this will have
to be tackled. This series is the first in that direction.

Jason A. Donenfeld (8):
  net: introduce skb_list_walk_safe for skb segment walking
  net: tap: use skb_list_walk_safe helper for gso segments
  net: r8152: use skb_list_walk_safe helper for gso segments
  net: tg3: use skb_list_walk_safe helper for gso segments
  net: sunvnet: use skb_list_walk_safe helper for gso segments
  net: sfc: use skb_list_walk_safe helper for gso segments
  net: myri10ge: use skb_list_walk_safe helper for gso segments
  net: iwlwifi: use skb_list_walk_safe helper for gso segments

 drivers/net/ethernet/broadcom/tg3.c              | 12 +++++-------
 drivers/net/ethernet/myricom/myri10ge/myri10ge.c |  8 +++-----
 drivers/net/ethernet/sfc/tx.c                    |  7 ++-----
 drivers/net/ethernet/sun/sunvnet_common.c        |  9 +++------
 drivers/net/tap.c                                | 14 ++++++--------
 drivers/net/usb/r8152.c                          | 12 +++++-------
 drivers/net/wireguard/device.h                   |  8 --------
 drivers/net/wireless/intel/iwlwifi/mvm/tx.c      |  9 ++-------
 include/linux/skbuff.h                           |  5 +++++
 9 files changed, 31 insertions(+), 53 deletions(-)

Comments

David Miller Jan. 8, 2020, 11:21 p.m. UTC | #1
From: "Jason A. Donenfeld" <Jason@zx2c4.com>
Date: Wed,  8 Jan 2020 16:59:01 -0500

> This patchset introduces the skb_list_walk_safe helper macro, in order
> to add some sanity to the myrid ways drivers have of walking through gso
> segments. The goal is to reduce future bugs commonly caused by open
> coding these sorts of things, and to in the future make it easier to
> swap out the underlying list representation.
> 
> This first patch series addresses the easy uses of drivers iterating
> over the returned list of skb_gso_segments, for drivers that live in
> drivers/net/*. There are still other use cases to tackle later for
> net/*, and after these low-hanging fruits are taken care of, I imagine
> there are more subtle cases of gso segment walking that isn't just a
> direct return value from skb_gso_segments, and eventually this will have
> to be tackled. This series is the first in that direction.

I like this, applied to net-next and build testing.  Let's see where this
goes.

In the iwlwifi case, the skb_mark_not_on_list() is really redundant because
deep down inside the skb queue tail insert, __skb_insert() will unconditionally
always write both ->next and ->prev and there are no debugging checks along
the way which would trigger if skb->next was non-NULL.

I guess you could argue for defensive programming here, so there's that.

Anyways, thanks.