diff mbox series

[ovs-dev] Avoid crash in OvS while transmitting fragmented packets over tunnel.

Message ID 9AF8CC4A-9C37-4791-A7A9-C26AA91F5EAD@ericsson.com
State Accepted
Headers show
Series [ovs-dev] Avoid crash in OvS while transmitting fragmented packets over tunnel. | expand

Commit Message

Rohith Basavaraja April 20, 2018, 8:47 a.m. UTC 
Currently when fragmented packets are to be transmitted in to tunnel,
base_flow->nw_frag which was initially non-zero at reception is not
reset to zero when the base_flow and flow are rewritten
as part of the emulated tnl_push action in the ofproto-dpif-xlate
module.

Because of this when fragmented packets are transmitted out of tunnel,
we hit crash caused by the following assert.

lib/odp-util.c:5654: assertion flow->nw_proto == base_flow->nw_proto &&
flow->nw_frag == base_flow->nw_frag failed in commit_set_ipv4_action()

With the following change propagate_tunnel_data_to_flow__
is modified to reset *nw_frag* to zero. Also, that currently we don't
fragment tunnelled packets, we should reset *nw_frag* to zero in
propagate_tunnel_data_to_flow__.

Signed-off-by: Jan Scheurich <jan.scheurich@ericsson.com>
From: Rohith Basavaraja <rohith.basavaraja@ericsson.com>
CC: Jan Scheurich <jan.scheurich@ericsson.com>

---
 ofproto/ofproto-dpif-xlate.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Ben Pfaff May 9, 2018, 10:02 p.m. UTC | #1 
On Fri, Apr 20, 2018 at 08:47:58AM +0000, Rohith Basavaraja wrote:
> Currently when fragmented packets are to be transmitted in to tunnel,
> base_flow->nw_frag which was initially non-zero at reception is not
> reset to zero when the base_flow and flow are rewritten
> as part of the emulated tnl_push action in the ofproto-dpif-xlate
> module.
> 
> Because of this when fragmented packets are transmitted out of tunnel,
> we hit crash caused by the following assert.
> 
> lib/odp-util.c:5654: assertion flow->nw_proto == base_flow->nw_proto &&
> flow->nw_frag == base_flow->nw_frag failed in commit_set_ipv4_action()
> 
> With the following change propagate_tunnel_data_to_flow__
> is modified to reset *nw_frag* to zero. Also, that currently we don't
> fragment tunnelled packets, we should reset *nw_frag* to zero in
> propagate_tunnel_data_to_flow__.
> 
> Signed-off-by: Jan Scheurich <jan.scheurich@ericsson.com>
> From: Rohith Basavaraja <rohith.basavaraja@ericsson.com>
> CC: Jan Scheurich <jan.scheurich@ericsson.com>

Thanks, applied to master, branch-2.9, and branch-2.8.
Darrell Ball May 10, 2018, 8:45 p.m. UTC | #2 
Thanks Rohith
I see this patch was applied, but I have one question inline.


On 4/20/18, 1:48 AM, "ovs-dev-bounces@openvswitch.org on behalf of Rohith Basavaraja" <ovs-dev-bounces@openvswitch.org on behalf of rohith.basavaraja@ericsson.com> wrote:

    Currently when fragmented packets are to be transmitted in to tunnel,
    base_flow->nw_frag which was initially non-zero at reception is not
    reset to zero when the base_flow and flow are rewritten
    as part of the emulated tnl_push action in the ofproto-dpif-xlate
    module.
    
    Because of this when fragmented packets are transmitted out of tunnel,
    we hit crash caused by the following assert.
    
    lib/odp-util.c:5654: assertion flow->nw_proto == base_flow->nw_proto &&
    flow->nw_frag == base_flow->nw_frag failed in commit_set_ipv4_action()

Can you describe how you hit this assertion?
I have some testing in and around this code, but have not hit this yet, so I was curious?

    With the following change propagate_tunnel_data_to_flow__
    is modified to reset *nw_frag* to zero. 


    Also, that currently we don't
    fragment tunnelled packets, we should reset *nw_frag* to zero in
    propagate_tunnel_data_to_flow__.

    Signed-off-by: Jan Scheurich <jan.scheurich@ericsson.com>
    From: Rohith Basavaraja <rohith.basavaraja@ericsson.com>
    CC: Jan Scheurich <jan.scheurich@ericsson.com>
    
    ---
     ofproto/ofproto-dpif-xlate.c | 1 +
     1 file changed, 1 insertion(+)
    
    diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
    index 94e3ddb..e9ed037 100644
    --- a/ofproto/ofproto-dpif-xlate.c
    +++ b/ofproto/ofproto-dpif-xlate.c
    @@ -3310,6 +3310,7 @@ propagate_tunnel_data_to_flow__(struct flow *dst_flow,
         dst_flow->ipv6_dst = src_flow->tunnel.ipv6_dst;
         dst_flow->ipv6_src = src_flow->tunnel.ipv6_src;
     
    +    dst_flow->nw_frag = 0; /* Tunnel packets are unfragmented. */


         dst_flow->nw_tos = src_flow->tunnel.ip_tos;
         dst_flow->nw_ttl = src_flow->tunnel.ip_ttl;
         dst_flow->tp_dst = src_flow->tunnel.tp_dst;
    -- 
    1.9.1
    
    
    
    
    _______________________________________________
    dev mailing list
    dev@openvswitch.org
    https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=6ZMZ7J9yNmX0ZQRMQyUfQ8fZrhemcMFiUqpnVD_jN9w&s=lYu98hfGnEvKr7YcK50fxDDY9-d0mA3W0yYtpSeeIQo&e=
Rohith Basavaraja May 11, 2018, 6:35 a.m. UTC | #3 
Hi Darell,

I reproduce the issue by making VM to transmit fragmented packets and in OvS if we have corresponding rule
to transmit the received fragmented packet (from VM) over tunnel then I always hit the crash.

Thanks
Rohith

On 11/05/18, 2:16 AM, "Darrell Ball" <dball@vmware.com> wrote:

    Thanks Rohith
    I see this patch was applied, but I have one question inline.
    
    
    On 4/20/18, 1:48 AM, "ovs-dev-bounces@openvswitch.org on behalf of Rohith Basavaraja" <ovs-dev-bounces@openvswitch.org on behalf of rohith.basavaraja@ericsson.com> wrote:
    
        Currently when fragmented packets are to be transmitted in to tunnel,
        base_flow->nw_frag which was initially non-zero at reception is not
        reset to zero when the base_flow and flow are rewritten
        as part of the emulated tnl_push action in the ofproto-dpif-xlate
        module.
        
        Because of this when fragmented packets are transmitted out of tunnel,
        we hit crash caused by the following assert.
        
        lib/odp-util.c:5654: assertion flow->nw_proto == base_flow->nw_proto &&
        flow->nw_frag == base_flow->nw_frag failed in commit_set_ipv4_action()
    
    Can you describe how you hit this assertion?
    I have some testing in and around this code, but have not hit this yet, so I was curious?
    
        With the following change propagate_tunnel_data_to_flow__
        is modified to reset *nw_frag* to zero. 
    
    
        Also, that currently we don't
        fragment tunnelled packets, we should reset *nw_frag* to zero in
        propagate_tunnel_data_to_flow__.
    
        Signed-off-by: Jan Scheurich <jan.scheurich@ericsson.com>
        From: Rohith Basavaraja <rohith.basavaraja@ericsson.com>
        CC: Jan Scheurich <jan.scheurich@ericsson.com>
        
        ---
         ofproto/ofproto-dpif-xlate.c | 1 +
         1 file changed, 1 insertion(+)
        
        diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
        index 94e3ddb..e9ed037 100644
        --- a/ofproto/ofproto-dpif-xlate.c
        +++ b/ofproto/ofproto-dpif-xlate.c
        @@ -3310,6 +3310,7 @@ propagate_tunnel_data_to_flow__(struct flow *dst_flow,
             dst_flow->ipv6_dst = src_flow->tunnel.ipv6_dst;
             dst_flow->ipv6_src = src_flow->tunnel.ipv6_src;
         
        +    dst_flow->nw_frag = 0; /* Tunnel packets are unfragmented. */
    
    
             dst_flow->nw_tos = src_flow->tunnel.ip_tos;
             dst_flow->nw_ttl = src_flow->tunnel.ip_ttl;
             dst_flow->tp_dst = src_flow->tunnel.tp_dst;
        -- 
        1.9.1
        
        
        
        
        _______________________________________________
        dev mailing list
        dev@openvswitch.org
        https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=6ZMZ7J9yNmX0ZQRMQyUfQ8fZrhemcMFiUqpnVD_jN9w&s=lYu98hfGnEvKr7YcK50fxDDY9-d0mA3W0yYtpSeeIQo&e=
Darrell Ball May 11, 2018, 5:02 p.m. UTC | #4 
On Thu, May 10, 2018 at 11:35 PM, Rohith Basavaraja <
rohith.basavaraja@ericsson.com> wrote:

> Hi Darell,
>
> I reproduce the issue by making VM to transmit fragmented packets and in
> OvS if we have corresponding rule
> to transmit the received fragmented packet (from VM) over tunnel then I
> always hit the crash.
>
>
Thanks Rohith

Unfortunately, I don't hit the assert.
I even tried again with the latest master with this change removed.
I used Geneve for the last test, but I am pretty sure I used Vxlan as well
for this kind of test b4, as I use it often as well.

Would you mind describing your test in more detail (packets and flows)?

Thanks Darrell





> Thanks
> Rohith
>
> On 11/05/18, 2:16 AM, "Darrell Ball" <dball@vmware.com> wrote:
>
>     Thanks Rohith
>     I see this patch was applied, but I have one question inline.
>
>
>     On 4/20/18, 1:48 AM, "ovs-dev-bounces@openvswitch.org on behalf of
> Rohith Basavaraja" <ovs-dev-bounces@openvswitch.org on behalf of
> rohith.basavaraja@ericsson.com> wrote:
>
>         Currently when fragmented packets are to be transmitted in to
> tunnel,
>         base_flow->nw_frag which was initially non-zero at reception is not
>         reset to zero when the base_flow and flow are rewritten
>         as part of the emulated tnl_push action in the ofproto-dpif-xlate
>         module.
>
>         Because of this when fragmented packets are transmitted out of
> tunnel,
>         we hit crash caused by the following assert.
>
>         lib/odp-util.c:5654: assertion flow->nw_proto ==
> base_flow->nw_proto &&
>         flow->nw_frag == base_flow->nw_frag failed in
> commit_set_ipv4_action()
>
>     Can you describe how you hit this assertion?
>     I have some testing in and around this code, but have not hit this
> yet, so I was curious?
>
>         With the following change propagate_tunnel_data_to_flow__
>         is modified to reset *nw_frag* to zero.
>
>
>         Also, that currently we don't
>         fragment tunnelled packets, we should reset *nw_frag* to zero in
>         propagate_tunnel_data_to_flow__.
>
>         Signed-off-by: Jan Scheurich <jan.scheurich@ericsson.com>
>         From: Rohith Basavaraja <rohith.basavaraja@ericsson.com>
>         CC: Jan Scheurich <jan.scheurich@ericsson.com>
>
>         ---
>          ofproto/ofproto-dpif-xlate.c | 1 +
>          1 file changed, 1 insertion(+)
>
>         diff --git a/ofproto/ofproto-dpif-xlate.c
> b/ofproto/ofproto-dpif-xlate.c
>         index 94e3ddb..e9ed037 100644
>         --- a/ofproto/ofproto-dpif-xlate.c
>         +++ b/ofproto/ofproto-dpif-xlate.c
>         @@ -3310,6 +3310,7 @@ propagate_tunnel_data_to_flow__(struct flow
> *dst_flow,
>              dst_flow->ipv6_dst = src_flow->tunnel.ipv6_dst;
>              dst_flow->ipv6_src = src_flow->tunnel.ipv6_src;
>
>         +    dst_flow->nw_frag = 0; /* Tunnel packets are unfragmented. */
>
>
>              dst_flow->nw_tos = src_flow->tunnel.ip_tos;
>              dst_flow->nw_ttl = src_flow->tunnel.ip_ttl;
>              dst_flow->tp_dst = src_flow->tunnel.tp_dst;
>         --
>         1.9.1
>
>
>
>
>         _______________________________________________
>         dev mailing list
>         dev@openvswitch.org
>         https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.
> openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=
> uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=
> 6ZMZ7J9yNmX0ZQRMQyUfQ8fZrhemcMFiUqpnVD_jN9w&s=lYu98hfGnEvKr7YcK50fxDDY9-
> d0mA3W0yYtpSeeIQo&e=
>
>
>
>
>
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
Rohith Basavaraja May 14, 2018, 10:36 a.m. UTC | #5 
Hi Darell,

In my simple setup I have interface in namespace ns1 with IP 192.168.10.10 and in connected to bridge br-in1 (Just mentioning relevant config)
And br-in1 I have following flows.

lab@ubuntu-vm:/var/log/openvswitch$ sudo ovs-ofctl -OOpenFlow13 dump-flows br-in1
 cookie=0x0, duration=83.662s, table=0, n_packets=75, n_bytes=7294, in_port="br-in1-ns1" actions=push_mpls:0x8847,set_field:666->mpls_label,output:gre1
 cookie=0x0, duration=83.644s, table=0, n_packets=75, n_bytes=7594, mpls,in_port=gre1,mpls_label=999 actions=pop_mpls:0x0800,set_field:26:ed:b2:5d:50:d7->eth_dst,output:"br-in1-ns1"

When I ping the remote interface things works fine

lab@ubuntu-vm:~$ sudo ip netns exec ns2 ping -W 1 192.168.10.10
PING 192.168.10.10 (192.168.10.10) 56(84) bytes of data.
64 bytes from 192.168.10.10: icmp_seq=1 ttl=64 time=1.62 ms
64 bytes from 192.168.10.10: icmp_seq=2 ttl=64 time=0.723 ms

and the corresponding dpctl entries are are as follows

lab@ubuntu-vm:~/testsuites$ sudo ovs-appctl dpif/dump-flows br-in1

tunnel(src=10.0.0.2,dst=10.0.0.1,flags(-df-csum)),recirc_id(0),in_port(7),packet_type(ns=0,id=0),eth(dst=26:ed:b2:5d:50:d7),eth_type(0x8847),mpls(label=999/0xfffff,tc=0/0,ttl=64/0x0,bos=1/1), packets:33, bytes:3366, used:0.978s, actions:pop_mpls(eth_type=0x800),5

recirc_id(0),in_port(5),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(tos=0/0xfc,ttl=64,frag=no), packets:33, bytes:3234, used:0.978s, actions:push_mpls(label=666,tc=0,ttl=64,bos=1,eth_type=0x8847),clone(tnl_push(tnl_port(7),header(size=38,type=3,eth(dst=86:67:03:ad:e7:e9,src=7a:9f:1f:ce:fc:b6,dl_type=0x0800),ipv4(src=10.0.0.1,dst=10.0.0.2,proto=47,tos=0,ttl=64,frag=0x4000),gre((flags=0x0,proto=0x6558))),out_port(2)),8)


Then Moments I start transmitting the fragmented packet (by increasing the packet size) I hit the assert mentioned in the problem.

sudo ip netns exec ns2 ping -s 5000 -W 1 192.168.10.10 <=== Note packet size is increased to force fragmentation at the sender.

2018-05-14T18:35:44.571Z|00114|util|EMER|lib/odp-util.c:7008: assertion flow->nw_proto == base_flow->nw_proto && flow->nw_frag == base_flow->nw_frag failed in commit_set_ipv4_action()
2018-05-14T18:35:44.835Z|00002|daemon_unix(monitor)|ERR|1 crashes: pid 19200 died, killed (Aborted), core dumped, restarting

Thanks
Rohith

From: Darrell Ball <dlu998@gmail.com>
Date: Friday, 11 May 2018 at 10:33 PM
To: Rohith Basavaraja <rohith.basavaraja@ericsson.com>
Cc: Darrell Ball <dball@vmware.com>, "dev@openvswitch.org" <dev@openvswitch.org>
Subject: Re: [ovs-dev] [PATCH] Avoid crash in OvS while transmitting fragmented packets over tunnel.



On Thu, May 10, 2018 at 11:35 PM, Rohith Basavaraja <rohith.basavaraja@ericsson.com<mailto:rohith.basavaraja@ericsson.com>> wrote:
Hi Darell,

I reproduce the issue by making VM to transmit fragmented packets and in OvS if we have corresponding rule
to transmit the received fragmented packet (from VM) over tunnel then I always hit the crash.

Thanks Rohith

Unfortunately, I don't hit the assert.
I even tried again with the latest master with this change removed.
I used Geneve for the last test, but I am pretty sure I used Vxlan as well for this kind of test b4, as I use it often as well.

Would you mind describing your test in more detail (packets and flows)?

Thanks Darrell




Thanks
Rohith

On 11/05/18, 2:16 AM, "Darrell Ball" <dball@vmware.com<mailto:dball@vmware.com>> wrote:

    Thanks Rohith
    I see this patch was applied, but I have one question inline.


    On 4/20/18, 1:48 AM, "ovs-dev-bounces@openvswitch.org<mailto:ovs-dev-bounces@openvswitch.org> on behalf of Rohith Basavaraja" <ovs-dev-bounces@openvswitch.org<mailto:ovs-dev-bounces@openvswitch.org> on behalf of rohith.basavaraja@ericsson.com<mailto:rohith.basavaraja@ericsson.com>> wrote:

        Currently when fragmented packets are to be transmitted in to tunnel,
        base_flow->nw_frag which was initially non-zero at reception is not
        reset to zero when the base_flow and flow are rewritten
        as part of the emulated tnl_push action in the ofproto-dpif-xlate
        module.

        Because of this when fragmented packets are transmitted out of tunnel,
        we hit crash caused by the following assert.

        lib/odp-util.c:5654: assertion flow->nw_proto == base_flow->nw_proto &&
        flow->nw_frag == base_flow->nw_frag failed in commit_set_ipv4_action()

    Can you describe how you hit this assertion?
    I have some testing in and around this code, but have not hit this yet, so I was curious?

        With the following change propagate_tunnel_data_to_flow__
        is modified to reset *nw_frag* to zero.


        Also, that currently we don't
        fragment tunnelled packets, we should reset *nw_frag* to zero in
        propagate_tunnel_data_to_flow__.

        Signed-off-by: Jan Scheurich <jan.scheurich@ericsson.com<mailto:jan.scheurich@ericsson.com>>
        From: Rohith Basavaraja <rohith.basavaraja@ericsson.com<mailto:rohith.basavaraja@ericsson.com>>
        CC: Jan Scheurich <jan.scheurich@ericsson.com<mailto:jan.scheurich@ericsson.com>>

        ---
         ofproto/ofproto-dpif-xlate.c | 1 +
         1 file changed, 1 insertion(+)

        diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
        index 94e3ddb..e9ed037 100644
        --- a/ofproto/ofproto-dpif-xlate.c
        +++ b/ofproto/ofproto-dpif-xlate.c
        @@ -3310,6 +3310,7 @@ propagate_tunnel_data_to_flow__(struct flow *dst_flow,
             dst_flow->ipv6_dst = src_flow->tunnel.ipv6_dst;
             dst_flow->ipv6_src = src_flow->tunnel.ipv6_src;

        +    dst_flow->nw_frag = 0; /* Tunnel packets are unfragmented. */


             dst_flow->nw_tos = src_flow->tunnel.ip_tos;
             dst_flow->nw_ttl = src_flow->tunnel.ip_ttl;
             dst_flow->tp_dst = src_flow->tunnel.tp_dst;
        --
        1.9.1




        _______________________________________________
        dev mailing list
        dev@openvswitch.org<mailto:dev@openvswitch.org>
        https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=6ZMZ7J9yNmX0ZQRMQyUfQ8fZrhemcMFiUqpnVD_jN9w&s=lYu98hfGnEvKr7YcK50fxDDY9-d0mA3W0yYtpSeeIQo&e=
Darrell Ball May 16, 2018, 4:56 p.m. UTC | #6 
Thanks Rohith



MPLSoGRE was not among my top 10 guesses :-); I’ll take a look.



Darrell


On Mon, May 14, 2018 at 3:36 AM, Rohith Basavaraja <
rohith.basavaraja@ericsson.com> wrote:

> Hi Darell,
>
>
>
> In my simple setup I have interface in namespace ns1 with IP 192.168.10.10
> and in connected to bridge br-in1 (Just mentioning relevant config)
>
> And br-in1 I have following flows.
>
>
>
> lab@ubuntu-vm:/var/log/openvswitch$ sudo ovs-ofctl -OOpenFlow13
> dump-flows br-in1
>
>  cookie=0x0, duration=83.662s, table=0, n_packets=75, n_bytes=7294,
> in_port="br-in1-ns1" actions=push_mpls:0x8847,set_
> field:666->mpls_label,output:gre1
>
>  cookie=0x0, duration=83.644s, table=0, n_packets=75, n_bytes=7594,
> mpls,in_port=gre1,mpls_label=999 actions=pop_mpls:0x0800,set_
> field:26:ed:b2:5d:50:d7->eth_dst,output:"br-in1-ns1"
>
>
>
> When I ping the remote interface things works fine
>
>
>
> lab@ubuntu-vm:~$ sudo ip netns exec ns2 ping -W 1 192.168.10.10
>
> PING 192.168.10.10 (192.168.10.10) 56(84) bytes of data.
>
> 64 bytes from 192.168.10.10: icmp_seq=1 ttl=64 time=1.62 ms
>
> 64 bytes from 192.168.10.10: icmp_seq=2 ttl=64 time=0.723 ms
>
>
>
> and the corresponding dpctl entries are are as follows
>
>
>
> lab@ubuntu-vm:~/testsuites$ sudo ovs-appctl dpif/dump-flows br-in1
>
>
>
> tunnel(src=10.0.0.2,dst=10.0.0.1,flags(-df-csum)),recirc_
> id(0),in_port(7),packet_type(ns=0,id=0),eth(dst=26:ed:b2:
> 5d:50:d7),eth_type(0x8847),mpls(label=999/0xfffff,tc=0/0,ttl=64/0x0,bos=1/1),
> packets:33, bytes:3366, used:0.978s, actions:pop_mpls(eth_type=0x800),5
>
>
>
> recirc_id(0),in_port(5),packet_type(ns=0,id=0),eth_
> type(0x0800),ipv4(tos=0/0xfc,ttl=64,frag=no), packets:33, bytes:3234,
> used:0.978s, actions:push_mpls(label=666,tc=0,ttl=64,bos=1,eth_type=
> 0x8847),clone(tnl_push(tnl_port(7),header(size=38,type=3,
> eth(dst=86:67:03:ad:e7:e9,src=7a:9f:1f:ce:fc:b6,dl_type=
> 0x0800),ipv4(src=10.0.0.1,dst=10.0.0.2,proto=47,tos=0,ttl=
> 64,frag=0x4000),gre((flags=0x0,proto=0x6558))),out_port(2)),8)
>
>
>
>
>
> Then Moments I start transmitting the fragmented packet (by increasing the
> packet size) I hit the assert mentioned in the problem.
>
>
>
> sudo ip netns exec ns2 ping -s 5000 -W 1 192.168.10.10 ç= Note packet
> size is increased to force fragmentation at the sender.
>
>
>
> 2018-05-14T18:35:44.571Z|00114|util|EMER|lib/odp-util.c:7008: assertion
> flow->nw_proto == base_flow->nw_proto && flow->nw_frag ==
> base_flow->nw_frag failed in commit_set_ipv4_action()
>
> 2018-05-14T18:35:44.835Z|00002|daemon_unix(monitor)|ERR|1 crashes: pid
> 19200 died, killed (Aborted), core dumped, restarting
>
>
>
> Thanks
>
> Rohith
>
>
>
> *From: *Darrell Ball <dlu998@gmail.com>
> *Date: *Friday, 11 May 2018 at 10:33 PM
> *To: *Rohith Basavaraja <rohith.basavaraja@ericsson.com>
> *Cc: *Darrell Ball <dball@vmware.com>, "dev@openvswitch.org" <
> dev@openvswitch.org>
> *Subject: *Re: [ovs-dev] [PATCH] Avoid crash in OvS while transmitting
> fragmented packets over tunnel.
>
>
>
>
>
>
>
> On Thu, May 10, 2018 at 11:35 PM, Rohith Basavaraja <
> rohith.basavaraja@ericsson.com> wrote:
>
> Hi Darell,
>
> I reproduce the issue by making VM to transmit fragmented packets and in
> OvS if we have corresponding rule
> to transmit the received fragmented packet (from VM) over tunnel then I
> always hit the crash.
>
>
>
> Thanks Rohith
>
>
>
> Unfortunately, I don't hit the assert.
>
> I even tried again with the latest master with this change removed.
>
> I used Geneve for the last test, but I am pretty sure I used Vxlan as well
> for this kind of test b4, as I use it often as well.
>
>
>
> Would you mind describing your test in more detail (packets and flows)?
>
>
>
> Thanks Darrell
>
>
>
>
>
>
>
>
>
> Thanks
> Rohith
>
>
> On 11/05/18, 2:16 AM, "Darrell Ball" <dball@vmware.com> wrote:
>
>     Thanks Rohith
>     I see this patch was applied, but I have one question inline.
>
>
>     On 4/20/18, 1:48 AM, "ovs-dev-bounces@openvswitch.org on behalf of
> Rohith Basavaraja" <ovs-dev-bounces@openvswitch.org on behalf of
> rohith.basavaraja@ericsson.com> wrote:
>
>         Currently when fragmented packets are to be transmitted in to
> tunnel,
>         base_flow->nw_frag which was initially non-zero at reception is not
>         reset to zero when the base_flow and flow are rewritten
>         as part of the emulated tnl_push action in the ofproto-dpif-xlate
>         module.
>
>         Because of this when fragmented packets are transmitted out of
> tunnel,
>         we hit crash caused by the following assert.
>
>         lib/odp-util.c:5654: assertion flow->nw_proto ==
> base_flow->nw_proto &&
>         flow->nw_frag == base_flow->nw_frag failed in
> commit_set_ipv4_action()
>
>     Can you describe how you hit this assertion?
>     I have some testing in and around this code, but have not hit this
> yet, so I was curious?
>
>         With the following change propagate_tunnel_data_to_flow__
>         is modified to reset *nw_frag* to zero.
>
>
>         Also, that currently we don't
>         fragment tunnelled packets, we should reset *nw_frag* to zero in
>         propagate_tunnel_data_to_flow__.
>
>         Signed-off-by: Jan Scheurich <jan.scheurich@ericsson.com>
>         From: Rohith Basavaraja <rohith.basavaraja@ericsson.com>
>         CC: Jan Scheurich <jan.scheurich@ericsson.com>
>
>         ---
>          ofproto/ofproto-dpif-xlate.c | 1 +
>          1 file changed, 1 insertion(+)
>
>         diff --git a/ofproto/ofproto-dpif-xlate.c
> b/ofproto/ofproto-dpif-xlate.c
>         index 94e3ddb..e9ed037 100644
>         --- a/ofproto/ofproto-dpif-xlate.c
>         +++ b/ofproto/ofproto-dpif-xlate.c
>         @@ -3310,6 +3310,7 @@ propagate_tunnel_data_to_flow__(struct flow
> *dst_flow,
>              dst_flow->ipv6_dst = src_flow->tunnel.ipv6_dst;
>              dst_flow->ipv6_src = src_flow->tunnel.ipv6_src;
>
>         +    dst_flow->nw_frag = 0; /* Tunnel packets are unfragmented. */
>
>
>              dst_flow->nw_tos = src_flow->tunnel.ip_tos;
>              dst_flow->nw_ttl = src_flow->tunnel.ip_ttl;
>              dst_flow->tp_dst = src_flow->tunnel.tp_dst;
>         --
>         1.9.1
>
>
>
>
>         _______________________________________________
>         dev mailing list
>         dev@openvswitch.org
>         https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.
> openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=
> uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=
> 6ZMZ7J9yNmX0ZQRMQyUfQ8fZrhemcMFiUqpnVD_jN9w&s=lYu98hfGnEvKr7YcK50fxDDY9-
> d0mA3W0yYtpSeeIQo&e=
>
>
>
>
>
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
>
diff mbox series

Patch

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 94e3ddb..e9ed037 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3310,6 +3310,7 @@  propagate_tunnel_data_to_flow__(struct flow *dst_flow,
     dst_flow->ipv6_dst = src_flow->tunnel.ipv6_dst;
     dst_flow->ipv6_src = src_flow->tunnel.ipv6_src;
 
+    dst_flow->nw_frag = 0; /* Tunnel packets are unfragmented. */
     dst_flow->nw_tos = src_flow->tunnel.ip_tos;
     dst_flow->nw_ttl = src_flow->tunnel.ip_ttl;
     dst_flow->tp_dst = src_flow->tunnel.tp_dst;