On Wed, Apr 16, 2014 at 11:03 AM, Pravin Shelar <[email protected]> wrote: > On Wed, Apr 16, 2014 at 6:51 AM, Andy Zhou <[email protected]> wrote: >> Recirculation implementation for Linux kernel data path. >> >> Signed-off-by: Andy Zhou <[email protected]> >> --- >> 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) _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
