[ovs-dev] [recirc datapath V2 3/3] datapath: add recirc action

Pravin Shelar pshelar at nicira.com
Wed Apr 16 18:03:34 UTC 2014


On Wed, Apr 16, 2014 at 6:51 AM, Andy Zhou <azhou at nicira.com> wrote:
> Recirculation implementation for Linux kernel data path.
>
> Signed-off-by: Andy Zhou <azhou at nicira.com>
> ---
>  datapath/actions.c      | 49 ++++++++++++++++++++++++++++++++++++++++++++++++-
>  datapath/datapath.c     | 37 ++++++++++++++++++++++---------------
>  datapath/datapath.h     |  4 +++-
>  datapath/flow.h         |  1 +
>  datapath/flow_netlink.c | 30 +++++++++++++++++++++++++++++-
>  5 files changed, 103 insertions(+), 18 deletions(-)
>
> diff --git a/datapath/actions.c b/datapath/actions.c
> index cb239c8..4182b3d 100644
> --- a/datapath/actions.c
> +++ b/datapath/actions.c
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2007-2013 Nicira, Inc.
> + * Copyright (c) 2007-2014 Nicira, Inc.
>   *
>   * This program is free software; you can redistribute it and/or
>   * modify it under the terms of version 2 of the GNU General Public
> @@ -521,6 +521,34 @@ static int execute_set_action(struct sk_buff *skb,
>         return err;
>  }
>
> +static int execute_recirc(struct datapath *dp, struct sk_buff *skb,
> +                                const struct nlattr *a)
> +{
> +       struct sw_flow_key recirc_key;
> +       uint16_t in_port = OVS_CB(skb)->pkt_key->phy.in_port;
> +       uint32_t dp_hash = OVS_CB(skb)->pkt_key->dp_hash;
> +       struct vport *p;
> +       int err;
> +
> +       err = ovs_flow_extract(skb, in_port, &recirc_key);
> +       if (err)
> +               return err;
> +
> +       p = ovs_vport_rcu(dp, in_port);
> +       if (unlikely(!p)) {
> +               kfree_skb(skb);
> +               return -ENODEV;
> +       }
> +
We can store in-port in ovs-cb to avoid hash lookup.

> +       recirc_key.dp_hash = dp_hash;
> +       recirc_key.recirc_id = nla_get_u32(a);
> +       OVS_CB(skb)->pkt_key = &recirc_key;
> +
Now there is no need to set key here.

> +       ovs_dp_process_packet_with_key(p, skb, &recirc_key);
> +
> +       return 0;
> +}
> +
>  /* Execute a list of actions against 'skb'. */
>  static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
>                         const struct nlattr *attr, int len, bool keep_skb)
> @@ -565,6 +593,25 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
>                         err = pop_vlan(skb);
>                         break;
>
> +               case OVS_ACTION_ATTR_RECIRC: {
> +                       struct sk_buff *recirc_skb;
> +                       const bool last_action = (a->nla_len == rem);
> +
> +                       if (!last_action || keep_skb)
> +                               recirc_skb = skb_clone(skb, GFP_ATOMIC);
> +                       else
> +                               recirc_skb = skb;
> +
> +                       err = execute_recirc(dp, recirc_skb, a);
> +
> +                       /* Return directly if recirc is the last action
> +                        * or err.  */
> +                       if (last_action || err)
> +                               return err;
> +
> +                       break;
> +               }
> +
>                 case OVS_ACTION_ATTR_SET:
>                         err = execute_set_action(skb, nla_data(a));
>                         break;
> diff --git a/datapath/datapath.c b/datapath/datapath.c
> index 6dfe69d..267daf9 100644
> --- a/datapath/datapath.c
> +++ b/datapath/datapath.c
> @@ -240,33 +240,25 @@ void ovs_dp_detach_port(struct vport *p)
>         ovs_vport_del(p);
>  }
>
> -/* Must be called with rcu_read_lock. */
> -void ovs_dp_process_received_packet(struct vport *p, struct sk_buff *skb)
> +void ovs_dp_process_packet_with_key(const struct vport *p,
> +               struct sk_buff *skb, struct sw_flow_key *pkt_key)
>  {
>         struct datapath *dp = p->dp;
>         struct sw_flow *flow;
>         struct dp_stats_percpu *stats;
> -       struct sw_flow_key key;
>         u64 *stats_counter;
>         u32 n_mask_hit;
> -       int error;
>
> +       OVS_CB(skb)->pkt_key = pkt_key;
we can group this with flow assignment.

>         stats = this_cpu_ptr(dp->stats_percpu);
>
> -       /* Extract flow from 'skb' into 'key'. */
> -       error = ovs_flow_extract(skb, p->port_no, &key);
> -       if (unlikely(error)) {
> -               kfree_skb(skb);
> -               return;
> -       }
> -
>         /* Look up flow. */
> -       flow = ovs_flow_tbl_lookup_stats(&dp->table, &key, &n_mask_hit);
> +       flow = ovs_flow_tbl_lookup_stats(&dp->table, pkt_key, &n_mask_hit);
>         if (unlikely(!flow)) {
>                 struct dp_upcall_info upcall;
>
>                 upcall.cmd = OVS_PACKET_CMD_MISS;
> -               upcall.key = &key;
> +               upcall.key = pkt_key;
>                 upcall.userdata = NULL;
>                 upcall.portid = ovs_vport_find_upcall_portid(p, skb);
>                 ovs_dp_upcall(dp, skb, &upcall);
> @@ -276,9 +268,8 @@ void ovs_dp_process_received_packet(struct vport *p, struct sk_buff *skb)
>         }
>
>         OVS_CB(skb)->flow = flow;
> -       OVS_CB(skb)->pkt_key = &key;
>
> -       ovs_flow_stats_update(OVS_CB(skb)->flow, key.tp.flags, skb);
> +       ovs_flow_stats_update(OVS_CB(skb)->flow, pkt_key->tp.flags, skb);
>         ovs_execute_actions(dp, skb);
>         stats_counter = &stats->n_hit;
>
> @@ -290,6 +281,22 @@ out:
>         u64_stats_update_end(&stats->sync);
>  }
>
> +/* Must be called with rcu_read_lock. */
> +void ovs_dp_process_received_packet(struct vport *p, struct sk_buff *skb)
> +{
> +       int error;
> +       struct sw_flow_key key;
> +
> +       /* Extract flow from 'skb' into 'key'. */
> +       error = ovs_flow_extract(skb, p->port_no, &key);
> +       if (unlikely(error)) {
> +               kfree_skb(skb);
> +               return;
> +       }
> +
> +       ovs_dp_process_packet_with_key(p, skb, &key);
> +}
> +
>  int ovs_dp_upcall(struct datapath *dp, struct sk_buff *skb,
>                   const struct dp_upcall_info *upcall_info)
>  {
> diff --git a/datapath/datapath.h b/datapath/datapath.h
> index 40e0f90..526ea91 100644
> --- a/datapath/datapath.h
> +++ b/datapath/datapath.h
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2007-2012 Nicira, Inc.
> + * Copyright (c) 2007-2014 Nicira, Inc.
>   *
>   * This program is free software; you can redistribute it and/or
>   * modify it under the terms of version 2 of the GNU General Public
> @@ -188,6 +188,8 @@ extern struct genl_family dp_vport_genl_family;
>  extern struct genl_multicast_group ovs_dp_vport_multicast_group;
>
>  void ovs_dp_process_received_packet(struct vport *, struct sk_buff *);
> +void ovs_dp_process_packet_with_key(const struct vport *, struct sk_buff *,
> +               struct sw_flow_key *pkt_key);
>  void ovs_dp_detach_port(struct vport *);
>  int ovs_dp_upcall(struct datapath *, struct sk_buff *,
>                   const struct dp_upcall_info *);
> diff --git a/datapath/flow.h b/datapath/flow.h
> index bcc36d2..f113b9a 100644
> --- a/datapath/flow.h
> +++ b/datapath/flow.h
> @@ -75,6 +75,7 @@ struct sw_flow_key {
>                 u16     in_port;        /* Input switch port (or DP_MAX_PORTS). */
>         } __packed phy; /* Safe when right after 'tun_key'. */
>         u32 dp_hash;                    /* Datapath computed hash value.  */
> +       u32 recirc_id;                  /* Recirculation ID.  */
>         struct {
>                 u8     src[ETH_ALEN];   /* Ethernet source address. */
>                 u8     dst[ETH_ALEN];   /* Ethernet destination address. */
> diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c
> index 0a1effa..883d9bf 100644
> --- a/datapath/flow_netlink.c
> +++ b/datapath/flow_netlink.c
> @@ -129,7 +129,8 @@ static bool match_validate(const struct sw_flow_match *match,
>         /* Always allowed mask fields. */
>         mask_allowed |= ((1ULL << OVS_KEY_ATTR_TUNNEL)
>                        | (1ULL << OVS_KEY_ATTR_IN_PORT)
> -                      | (1ULL << OVS_KEY_ATTR_ETHERTYPE));
> +                      | (1ULL << OVS_KEY_ATTR_ETHERTYPE)
> +                      | (1ULL << OVS_KEY_ATTR_RECIRC_ID));
>
>         /* Check key attributes. */
>         if (match->key->dp_hash) {
> @@ -258,6 +259,7 @@ static const int ovs_key_lens[OVS_KEY_ATTR_MAX + 1] = {
>         [OVS_KEY_ATTR_ARP] = sizeof(struct ovs_key_arp),
>         [OVS_KEY_ATTR_ND] = sizeof(struct ovs_key_nd),
>         [OVS_KEY_ATTR_DP_HASH] = sizeof(u32),
> +       [OVS_KEY_ATTR_RECIRC_ID] = sizeof(u32),
>         [OVS_KEY_ATTR_TUNNEL] = -1,
>  };
>
> @@ -474,6 +476,23 @@ static int metadata_from_nlattrs(struct sw_flow_match *match,  u64 *attrs,
>                 *attrs &= ~(1ULL << OVS_KEY_ATTR_DP_HASH);
>         }
>
> +       if (*attrs & (1ULL << OVS_KEY_ATTR_RECIRC_ID)) {
> +               u32 recirc_id = nla_get_u32(a[OVS_KEY_ATTR_RECIRC_ID]);
> +
> +               if (is_mask && (recirc_id > 0 && recirc_id < UINT_MAX)) {
> +                       OVS_NLERR("Reicrc_id mask is neither wildcard, nor exact match \n");
> +                       return -EINVAL;
> +               }
> +
> +               SW_FLOW_KEY_PUT(match, recirc_id, recirc_id, is_mask);
> +               *attrs &= ~(1ULL << OVS_KEY_ATTR_RECIRC_ID);
> +       }
> +
> +       if (is_mask) {
> +               /* Always exact match recirc_id. */
> +               SW_FLOW_KEY_PUT(match, recirc_id, UINT_MAX, is_mask);
> +       }
> +

If the recirc_id is zero it should be safe to ignore it. Can you
explain need for exact match for recirc_id ?



More information about the dev mailing list