Hi Ben and Daniele, Thanks for your tips. I submitted the patch again. The test has been added. May I also add “Signed-off-by: Daniele Di Proietto <[email protected]>” ?
Thanks. Nick > On Dec 7, 2016, at 7:51 AM, Daniele Di Proietto <[email protected]> wrote: > > Thanks for the patch > >>> --- >>> lib/dpif-netdev.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c >>> index 1400511..8175b7e 100644 >>> --- a/lib/dpif-netdev.c >>> +++ b/lib/dpif-netdev.c >>> @@ -4380,11 +4380,12 @@ dp_execute_userspace_action(struct >>> dp_netdev_pmd_thread *pmd, >>> const struct nlattr *userdata, long long now) >>> { >>> struct dp_packet_batch b; >>> + struct flow_wildcards wc; >>> int error; >>> >>> ofpbuf_clear(actions); >>> >>> - error = dp_netdev_upcall(pmd, packet, flow, NULL, ufid, >>> + error = dp_netdev_upcall(pmd, packet, flow, &wc, ufid, >>> DPIF_UC_ACTION, userdata, actions, >>> NULL); >>> if (!error || error == ENOSPC) { >> >> I'm not too familiar with dpif-netdev. However, there's probably some >> reason that this wasn't done to start with; for example, it might be a >> performance optimization of some kind. Therefore, it's probably best to >> at least consider fixing whatever function is doing the null >> dereference. > > I agree, it seems better to handle the case where 'wc' is NULL in upcall_cb(). > process_upcall() already handles it for non-miss upcalls, i.e. when coming > from > an OVS_ACTION_ATTR_USERSPACE datapath action. > > Also, I think we could add a test, like the following: _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
