[ovs-dev] [loops 4/6] datapath: Propagate packet transmit errors to execute_actions() caller.

Jesse Gross jesse at nicira.com
Mon Jul 26 23:11:03 UTC 2010


On Fri, Jul 23, 2010 at 4:44 PM, Ben Pfaff <blp at nicira.com> wrote:
>
> +static struct dp_port *output_group(struct datapath *dp, __u16 group,
> +                                   struct sk_buff *skb, gfp_t gfp)
>  {
> -       struct dp_port_group *g = rcu_dereference(dp->groups[group]);
> -       int prev_port = -1;
> +       struct dp_port_group *g;
> +       struct dp_port *prev_port;
>

I think you want to initialize prev_port to NULL.


>                case ODPAT_OUTPUT_GROUP:
>                        prev_port = output_group(dp, a->output_group.group,
>                                                 skb, gfp);
> +                       if (IS_ERR(prev_port))
> +                               return PTR_ERR(prev_port);
>

Isn't this going to leak the original skb in the event of an error?


> @@ -488,10 +499,10 @@ int execute_actions(struct datapath *dp, struct
> sk_buff *skb,
>                }
>                if (!skb)
>                        return -ENOMEM;
>

I think there's a missing closing brace for the for loop here.


> +       if (prev_port) {
> +               err = vport_send(prev_port->vport, skb);
> +               if (unlikely(err < 0))
> +                       return err;
>        }
> -       if (prev_port != -1)
> -               do_output(dp, skb, prev_port);
> -       else
> -               kfree_skb(skb);


Don't we need to free the skb if !prev_port?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20100726/3a5c3424/attachment-0003.html>


More information about the dev mailing list