Looking at do_output, it appears that we silently drop a packet if a bad out_port was specified because vport will be NULL from ovs_vport_rcu and we just free the skb and return from thereon. Instead, an ideal behavior should be to log this in user space via log_execute_message and also perhaps into dmesg as a warning.

In 2.3, we had do_output return an error, but it was never propagated to user space through do_execute_actions, but seems like fe90efd9d converted do_output to a 'void' function. I'm thinking if we should bring back return value of do_output and then have do_execute_actions return it to userspace.

Please suggest..

A patch on top of 2.3 should look like this:

diff --git a/datapath/actions.c b/datapath/actions.c
index ddacbc6..41b25aa 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -402,6 +402,7 @@ static int do_output(struct datapath *dp, struct sk_buff 
*skb, int out_port)
        vport = ovs_vport_rcu(dp, out_port);
        if (unlikely(!vport)) {
                kfree_skb(skb);
+               pr_warn("invalid port number specified: %d\n", out_port);
                return -ENODEV;
        }

@@ -584,13 +585,14 @@ static int do_execute_actions(struct datapath *dp, struct 
sk_buff *skb,
        int prev_port = -1;
        const struct nlattr *a;
        int rem;
+       int err;

        for (a = attr, rem = len; rem > 0;
             a = nla_next(a, &rem)) {
                int err = 0;

                if (prev_port != -1) {
-                       do_output(dp, skb_clone(skb, GFP_ATOMIC), prev_port);
+                       err = do_output(dp, skb_clone(skb, GFP_ATOMIC), 
prev_port);
                        prev_port = -1;
                }

@@ -651,11 +653,11 @@ static int do_execute_actions(struct datapath *dp, struct 
sk_buff *skb,
        }

        if (prev_port != -1)
-               do_output(dp, skb, prev_port);
+               err = do_output(dp, skb, prev_port);
        else
                consume_skb(skb);

-       return 0;
+       return err;
 }

 /* We limit the number of times that we pass into execute_actions()

_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to