On 12 Jul 2019, at 8:29, Edward Cree wrote:
On 10/07/2019 23:47, Sabrina Dubroca wrote:
2019-07-10, 16:07:43 +0100, Edward Cree wrote:
On 10/07/2019 14:52, Sabrina Dubroca wrote:
-static int __netif_receive_skb_core(struct sk_buff *skb, bool
pfmemalloc,
+static int __netif_receive_skb_core(struct sk_buff **pskb, bool
pfmemalloc,
struct packet_type **ppt_prev)
{
struct packet_type *ptype, *pt_prev;
rx_handler_func_t *rx_handler;
+ struct sk_buff *skb = *pskb;
Would it not be simpler just to change all users of skb to *pskb?
Then you avoid having to keep doing "*pskb = skb;" whenever skb
changes
(with concomitant risk of bugs if one gets missed).
Yes, that would be less risky. I wrote a version of the patch that
did
exactly that, but found it really too ugly (both the patch and the
resulting code).
If you've still got that version (or can dig it out of your reflog),
can
you post it so we can see just how ugly it turns out?
We have more than 50 occurences of skb, including
things like:
atomic_long_inc(&skb->dev->rx_dropped);
Ooh, yes, I can see why that ends up looking funny...
Here's a thought, how about switching round the
return-vs-pass-by-pointer
and writing:
static struct sk_buff *__netif_receive_skb_core(struct sk_buff *skb,
bool pfmemalloc,
struct packet_type
**ppt_prev, int *ret)
?
(Although, you might want to rename 'ret' in that case.)
Does that make things any less ugly?
That was actually my first thought as well - this seems to fit well with
the
other APIS which can return a different skb.
--
Jonathan
