2016-12-07 10:04 GMT-08:00 nickcooper-zhangtonghao <n...@opencloud.tech>:
> When the datapath, whose type is "netdev", processes packets
> in userspce action, it may cause a segmentation fault. In the
> dp_execute_userspace_action(), we pass the "wc" argument to
> dp_netdev_upcall() using NULL. In the dp_netdev_upcall() call tree,
> the "wc" will be used. For example, dp_netdev_upcall() uses the
> &wc->masks for debugging, and flow_wildcards_init_for_packet()
> uses the  "wc" if we disable megaflow, which is described in
> more detail below.
>
> Segmentation fault in flow_wildcards_init_for_packet:
>
>     #0  0x0000000000468fe8 flow_wildcards_init_for_packet lib/flow.c:1275
>     #1  0x0000000000436c0b upcall_cb ofproto/ofproto-dpif-upcall.c:1231
>     #2  0x000000000045bd96 dp_netdev_upcall lib/dpif-netdev.c:3857
>     #3  0x0000000000461bf3 dp_execute_userspace_action lib/dpif-netdev.c:4388
>     #4  dp_execute_cb lib/dpif-netdev.c:4521
>     #5  0x0000000000486ae2 odp_execute_actions lib/odp-execute.c:538
>     #6  0x00000000004607f9 dp_netdev_execute_actions lib/dpif-netdev.c:4627
>     #7  packet_batch_per_flow_execute lib/dpif-netdev.c:3927
>     #8  dp_netdev_input__ lib/dpif-netdev.c:4229
>     #9  0x0000000000460ba8 dp_netdev_input lib/dpif-netdev.c:4238
>     #10 dp_netdev_process_rxq_port lib/dpif-netdev.c:2873
>     #11 0x000000000046126e dpif_netdev_run lib/dpif-netdev.c:3000
>     #12 0x000000000042baf5 type_run ofproto/ofproto-dpif.c:504
>     #13 0x00000000004192bf ofproto_type_run ofproto/ofproto.c:1687
>     #14 0x0000000000409965 bridge_run__ vswitchd/bridge.c:2875
>     #15 0x000000000040f145 bridge_run vswitchd/bridge.c:2938
>     #16 0x00000000004062e5 main vswitchd/ovs-vswitchd.c:111
>
> Signed-off-by: nickcooper-zhangtonghao <n...@opencloud.tech>
> Signed-off-by: Daniele Di Proietto <diproiet...@ovn.org>

Thanks!

I added another check for 'wc' in ukey_create_from_upcall().

I pushed this to master, branch-2.6 and branch-2.5

> ---
>  lib/dpif-netdev.c             |  2 +-
>  ofproto/ofproto-dpif-upcall.c |  4 ++--
>  tests/ofproto-dpif.at         | 34 ++++++++++++++++++++++++++++++++++
>  3 files changed, 37 insertions(+), 3 deletions(-)
>
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 1400511..0b73056 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -3834,7 +3834,7 @@ dp_netdev_upcall(struct dp_netdev_pmd_thread *pmd, 
> struct dp_packet *packet_,
>          struct ofpbuf key;
>          struct odp_flow_key_parms odp_parms = {
>              .flow = flow,
> -            .mask = &wc->masks,
> +            .mask = wc ? &wc->masks : NULL,
>              .support = dp_netdev_support,
>          };
>
> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
> index 6cb9c2e..aa7eabf 100644
> --- a/ofproto/ofproto-dpif-upcall.c
> +++ b/ofproto/ofproto-dpif-upcall.c
> @@ -1227,7 +1227,7 @@ upcall_cb(const struct dp_packet *packet, const struct 
> flow *flow, ovs_u128 *ufi
>                     upcall.put_actions.size);
>      }
>
> -    if (OVS_UNLIKELY(!megaflow)) {
> +    if (OVS_UNLIKELY(!megaflow && wc)) {
>          flow_wildcards_init_for_packet(wc, flow);
>      }
>
> @@ -1504,7 +1504,7 @@ ukey_create_from_upcall(struct upcall *upcall, struct 
> flow_wildcards *wc)
>      bool megaflow;
>      struct odp_flow_key_parms odp_parms = {
>          .flow = upcall->flow,
> -        .mask = &wc->masks,
> +        .mask = wc ? &wc->masks : NULL,
>      };
>
>      odp_parms.support = ofproto_dpif_get_support(upcall->ofproto)->odp;
> diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
> index cd90424..6d57445 100644
> --- a/tests/ofproto-dpif.at
> +++ b/tests/ofproto-dpif.at
> @@ -1627,6 +1627,40 @@ NXST_FLOW reply:
>  OVS_VSWITCHD_STOP
>  AT_CLEANUP
>
> +AT_SETUP([ofproto-dpif - controller action without megaflows])
> +OVS_VSWITCHD_START
> +add_of_ports br0 1
> +
> +AT_CHECK([ovs-ofctl add-flow br0 in_port=1,action=controller])
> +AT_CHECK([ovs-appctl upcall/disable-megaflows], [0], [dnl
> +megaflows disabled
> +])
> +
> +AT_CAPTURE_FILE([ofctl_monitor.log])
> +AT_CHECK([ovs-ofctl monitor br0 65534 invalid_ttl -P nxt_packet_in --detach 
> --no-chdir --pidfile 2> ofctl_monitor.log])
> +
> +for i in 1 2; do
> +    AT_CHECK([ovs-appctl netdev-dummy/receive p1 
> 'in_port(1),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x1234)'])
> +done
> +
> +OVS_WAIT_UNTIL([test `wc -l < ofctl_monitor.log` -ge 4])
> +OVS_WAIT_UNTIL([ovs-appctl -t ovs-ofctl exit])
> +
> +AT_CHECK([ovs-appctl dpctl/dump-flows | sed 's/.*\(packets:\)/\1/' | sed 
> 's/used:[[0-9]].[[0-9]]*s/used:0.001s/'], [0], [dnl
> +flow-dump from non-dpdk interfaces:
> +packets:1, bytes:14, used:0.001s, 
> actions:userspace(pid=0,slow_path(controller))
> +])
> +
> +AT_CHECK([cat ofctl_monitor.log], [0], [dnl
> +NXT_PACKET_IN (xid=0x0): cookie=0x0 total_len=14 in_port=1 (via action) 
> data_len=14 (unbuffered)
> +vlan_tci=0x0000,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,dl_type=0x1234
> +NXT_PACKET_IN (xid=0x0): cookie=0x0 total_len=14 in_port=1 (via action) 
> data_len=14 (unbuffered)
> +vlan_tci=0x0000,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,dl_type=0x1234
> +])
> +
> +OVS_VSWITCHD_STOP
> +AT_CLEANUP
> +
>  AT_SETUP([ofproto-dpif - MPLS handling])
>  OVS_VSWITCHD_START([dnl
>     add-port br0 p1 -- set Interface p1 type=dummy
> --
> 1.8.3.1
>
>
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to