[ovs-dev] [recirc datapath V2 3/3] datapath: add recirc action
Andy Zhou
azhou at nicira.com
Wed Apr 16 18:13:15 UTC 2014
On Wed, Apr 16, 2014 at 11:03 AM, Pravin Shelar <pshelar at nicira.com> wrote:
> 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.
Do you mean vport * ? Sure if ovs-cb can afford to lose 8 bytes.
>
>> + 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.
right, I missed it.
>
>> + 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.
O.K.
>
>> 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 ?
Consider the following two flows:
1) in_port = 1 ; actions= A
2) recirc_id = 2, in_port =1 : Actions=B
Without forcing exact match of recirc_id, a packet matches 2) could
also match 1)
More information about the dev
mailing list