On Thu, Oct 20, 2016 at 6:49 PM, Joe Stringer <j...@ovn.org> wrote:
> On 19 October 2016 at 11:27, Pravin B Shelar <pshe...@ovn.org> wrote:
>> On upstream kernel datapath OVS make use of networking devices
>> where networking stack does check if device is UP. following
>> patch adds same check in case of compat tunneling implementation.
>> This check also fixes kernel crash in case vxlan device was
>> brought down by user.
>>
>> CPU: 4 PID: 12988 Comm: handler903 Tainted:
>> RIP: 0010:[<ffffffffa05e5407>] vxlan_xmit_one.constprop.50+0x47/0x1210
>> [openvswitch]
>> Call Trace:
>> [<ffffffffa05e6625>] rpl_vxlan_xmit+0x55/0x80 [openvswitch]
>> [<ffffffffa05d5ad4>] ovs_vport_send+0x44/0xb0 [openvswitch]
>> [<ffffffffa05c62a5>] do_output+0x65/0x180 [openvswitch]
>> [<ffffffffa05c70dc>] do_execute_actions+0x10c/0x860 [openvswitch]
>> [<ffffffffa05c7870>] ovs_execute_actions+0x40/0x130 [openvswitch]
>> [<ffffffffa05cbb59>] ovs_packet_cmd_execute+0x2c9/0x2f0 [openvswitch]
>> [<ffffffff8155f31d>] genl_family_rcv_msg+0x1cd/0x400
>> [<ffffffff8155f5e1>] genl_rcv_msg+0x91/0xd0
>> [<ffffffff8155d549>] netlink_rcv_skb+0xa9/0xc0
>> [<ffffffff8155da78>] genl_rcv+0x28/0x40
>> [<ffffffff8155ceba>] netlink_unicast+0x16a/0x210
>> [<ffffffff8155d277>] netlink_sendmsg+0x317/0x430
>> [<ffffffff81514fd0>] sock_sendmsg+0xb0/0xf0
>> [<ffffffff81515409>] ___sys_sendmsg+0x3a9/0x3c0
>> [<ffffffff815162f1>] __sys_sendmsg+0x51/0x90
>> [<ffffffff81516342>] SyS_sendmsg+0x12/0x20
>> [<ffffffff81649609>] system_call_fastpath+0x16/0x1b
>>
>> Reported-by: Huanglili (lee) <huanglili.hu...@huawei.com>
>> Signed-off-by: Pravin B Shelar <pshe...@ovn.org>
>
> I think we need to use netif_running() for the check, not IFF_UP. in
> __dev_close_many(), it first clears the bit that is used by
> netif_running, then tries to deactive all of the tx queues,
> synchronizes with net if need be, calls ndo_stop(), then finally
> clears IFF_UP.
>
This works on close but during __dev_open() the flag
__LINK_STATE_START is set before calling ndo_open(). That does not
work for vxlan device.
I am trying to mimic upstream dev-queue-xmit behavior here. Therefore
we should check for IFF_UP flag.
>> ---
>> datapath/linux/compat/geneve.c | 6 ++++++
>> datapath/linux/compat/ip_gre.c | 3 +++
>> datapath/linux/compat/lisp.c | 5 +++++
>> datapath/linux/compat/stt.c | 8 +++++++-
>> datapath/linux/compat/vxlan.c | 5 ++++-
>> 5 files changed, 25 insertions(+), 2 deletions(-)
>>
>> diff --git a/datapath/linux/compat/geneve.c b/datapath/linux/compat/geneve.c
>> index 0c5b58a..1cd4f75 100644
>> --- a/datapath/linux/compat/geneve.c
>> +++ b/datapath/linux/compat/geneve.c
>> @@ -1118,6 +1118,12 @@ netdev_tx_t rpl_geneve_xmit(struct sk_buff *skb)
>> struct geneve_dev *geneve = netdev_priv(dev);
>> struct ip_tunnel_info *info = NULL;
>>
>> + if (unlikely(!(dev->flags & IFF_UP))) {
>> + dev->stats.tx_dropped++;
>> + kfree_skb(skb);
>> + return NETDEV_TX_OK;
>> + }
>> +
>
> I guess this is more like an 'expected' type of error, so we should
> use dev_kfree_skb()?
>
Thats true. I will post update patch.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
