[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CALnjE+oYj=X0Vij_0StjDo7Cd9Fg++uh8EtSmnvYe4u3tOFhcQ@mail.gmail.com>
Date: Wed, 26 Aug 2015 20:57:35 -0700
From: Pravin Shelar <pshelar@...ira.com>
To: Jesse Gross <jesse@...ira.com>
Cc: netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH v4 net-next 7/8] geneve: Consolidate Geneve functionality
in single module.
On Wed, Aug 26, 2015 at 8:08 PM, Jesse Gross <jesse@...ira.com> wrote:
> On Wed, Aug 26, 2015 at 2:54 PM, Pravin B Shelar <pshelar@...ira.com> wrote:
>> diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
>> index d05150c..a36a1de 100644
>> --- a/drivers/net/geneve.c
>> +++ b/drivers/net/geneve.c
>> @@ -138,16 +164,18 @@ static void geneve_rx(struct geneve_sock *gs, struct sk_buff *skb)
>> opts = ip_tunnel_info_opts(&tun_dst->u.tun_info,
>> gnvh->opt_len * 4);
>> memcpy(opts, gnvh->options, gnvh->opt_len * 4);
>> + xnet = false;
>> } else {
>> /* Drop packets w/ critical options,
>> * since we don't support any...
>> */
>> if (gnvh->critical)
>> goto drop;
>> + xnet = !net_eq(geneve->net, dev_net(geneve->dev));
>> }
>
> Now that devices can be placed in any namespace, I think we can't
> assume that xnet is false in the first case.
>
ok.
>> @@ -293,15 +615,13 @@ static netdev_tx_t geneve_xmit(struct sk_buff *skb, struct net_device *dev)
> [...]
>> + err = geneve_build_skb(rt, skb, key->tun_flags, vni,
>> + info->options_len, opts, udp_csum);
>> + if (unlikely(err))
>> + goto err;
> [...]
>> tx_error:
>> - dev->stats.tx_errors++;
>> dev_kfree_skb(skb);
>> +err:
>> + dev->stats.tx_errors++;
>> return NETDEV_TX_OK;
>> }
>
> I'm not sure that it makes sense to break out these error conditions -
> geneve_build_skb() doesn't free the skb, so it looks like there is a
> leak on failure.
It does frees skb. Which case are you looking at?
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists