[ovs-dev] [PATCH 3/3] datapath: Use tun_info only for egress tunnel path.
Pravin Shelar
pshelar at nicira.com
Wed Aug 6 22:45:16 UTC 2014
On Wed, Aug 6, 2014 at 3:30 PM, Andy Zhou <azhou at nicira.com> wrote:
> On Tue, Aug 5, 2014 at 4:46 PM, Pravin B Shelar <pshelar at nicira.com> wrote:
>> Currently tun_info is used for passing tunnel information
>> on ingress and egress path, this cause confusion. Following
>> patch removes its use on ingress path make it egress only parameter.
>
> Sorry for the patch rebasing request. I messed up on my end. It
> applies fine now.
>
> How should set tunnel info action be handled? It seems this
> information will be forgotten when execute recirculation.
>
if there are more output action after recirc, skb is cloned therefore
we do not loose this info.
> Current, we set OVS(skb)->tun_info to NULL before we execute actions.
> With your changes, would it make sense
> to set it to NULL at the flow extraction time?
>
ok, I can move it to extract.
>>
>> Signed-off-by: Pravin B Shelar <pshelar at nicira.com>
>> ---
>> datapath/actions.c | 2 +-
>> datapath/datapath.c | 23 +++--------------------
>> datapath/datapath.h | 8 +++-----
>> datapath/flow.c | 8 ++++----
>> datapath/flow.h | 3 ++-
>> datapath/vport.c | 11 +++++++++--
>> 6 files changed, 22 insertions(+), 33 deletions(-)
>>
>> diff --git a/datapath/actions.c b/datapath/actions.c
>> index 6de65d3..62d3ede 100644
>> --- a/datapath/actions.c
>> +++ b/datapath/actions.c
>> @@ -659,7 +659,7 @@ static int execute_recirc(struct datapath *dp, struct sk_buff *skb,
>> }
>>
>>
>> - ovs_dp_process_packet_with_key(skb, &recirc_key, true);
>> + ovs_dp_process_packet(skb, true);
>>
>> return 0;
>> }
>> diff --git a/datapath/datapath.c b/datapath/datapath.c
>> index 91c65a0..b662dd2 100644
>> --- a/datapath/datapath.c
>> +++ b/datapath/datapath.c
>> @@ -250,11 +250,11 @@ void ovs_dp_detach_port(struct vport *p)
>> ovs_vport_del(p);
>> }
>>
>> -void ovs_dp_process_packet_with_key(struct sk_buff *skb,
>> - struct sw_flow_key *pkt_key,
>> - bool recirc)
>> +/* Must be called with rcu_read_lock. */
>> +void ovs_dp_process_packet(struct sk_buff *skb, bool recirc)
>> {
>> const struct vport *p = OVS_CB(skb)->input_vport;
>> + struct sw_flow_key *pkt_key = OVS_CB(skb)->pkt_key;
>> struct datapath *dp = p->dp;
>> struct sw_flow *flow;
>> struct dp_stats_percpu *stats;
>> @@ -262,7 +262,6 @@ void ovs_dp_process_packet_with_key(struct sk_buff *skb,
>> u32 n_mask_hit;
>>
>> stats = this_cpu_ptr(dp->stats_percpu);
>> - OVS_CB(skb)->pkt_key = pkt_key;
>>
>> /* Look up flow. */
>> flow = ovs_flow_tbl_lookup_stats(&dp->table, pkt_key, skb_get_hash(skb),
>> @@ -293,22 +292,6 @@ out:
>> u64_stats_update_end(&stats->sync);
>> }
>>
>> -/* Must be called with rcu_read_lock. */
>> -void ovs_dp_process_received_packet(struct sk_buff *skb)
>> -{
>> - int error;
>> - struct sw_flow_key key;
>> -
>> - /* Extract flow from 'skb' into 'key'. */
>> - error = ovs_flow_key_extract(skb, &key);
>> - if (unlikely(error)) {
>> - kfree_skb(skb);
>> - return;
>> - }
>> -
>> - ovs_dp_process_packet_with_key(skb, &key, false);
>> -}
>> -
>> 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 fb37fa1..684ac51 100644
>> --- a/datapath/datapath.h
>> +++ b/datapath/datapath.h
>> @@ -98,8 +98,8 @@ struct datapath {
>> * struct ovs_skb_cb - OVS data in skb CB
>> * @flow: The flow associated with this packet. May be %NULL if no flow.
>> * @pkt_key: The flow information extracted from the packet. Must be nonnull.
>> - * @tun_info: Tunnel information about this packet. NULL if the packet
>> - * is not being tunneled.
>> + * @tun_info: Tunnel information about this packet on egress path. NULL if the
>> + * packet is not being tunneled.
>> * @input_vport: The original vport packet came in on. This value is cached
>> * when a packet is received by OVS.
>> */
>> @@ -188,9 +188,7 @@ extern struct notifier_block ovs_dp_device_notifier;
>> 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 sk_buff *);
>> -void ovs_dp_process_packet_with_key(struct sk_buff *,
>> - struct sw_flow_key *pkt_key, bool recirc);
>> +void ovs_dp_process_packet(struct sk_buff *, bool recirc);
>> 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.c b/datapath/flow.c
>> index 1feca85..00819e7 100644
>> --- a/datapath/flow.c
>> +++ b/datapath/flow.c
>> @@ -669,16 +669,16 @@ static int key_extract(struct sk_buff *skb, struct sw_flow_key *key)
>> }
>> }
>>
>> + OVS_CB(skb)->pkt_key = key;
>> return 0;
>> }
>>
>> -int ovs_flow_key_extract(struct sk_buff *skb, struct sw_flow_key *key)
>> +int ovs_flow_key_extract(struct ovs_tunnel_info *tun_info, struct sk_buff *skb,
>
> Should tun_info be const?
>
ok.
>> + struct sw_flow_key *key)
>> {
>> /* Extract metadata from packet. */
>>
>> - if (OVS_CB(skb)->tun_info) {
>> - struct ovs_tunnel_info *tun_info = OVS_CB(skb)->tun_info;
>> -
>> + if (tun_info) {
>> memcpy(&key->tun_key, &tun_info->tunnel, sizeof(key->tun_key));
>>
>> BUILD_BUG_ON(((1 << (sizeof(tun_info->options_len) * 8)) - 1) >
>> diff --git a/datapath/flow.h b/datapath/flow.h
>> index ee63b8b..30a3c55 100644
>> --- a/datapath/flow.h
>> +++ b/datapath/flow.h
>> @@ -222,7 +222,8 @@ void ovs_flow_stats_get(const struct sw_flow *, struct ovs_flow_stats *,
>> void ovs_flow_stats_clear(struct sw_flow *);
>> u64 ovs_flow_used_time(unsigned long flow_jiffies);
>>
>> -int ovs_flow_key_extract(struct sk_buff *skb, struct sw_flow_key *key);
>> +int ovs_flow_key_extract(struct ovs_tunnel_info *tun_info,
>> + struct sk_buff *skb, struct sw_flow_key *key);
>> /* Extract key from packet coming from userspace. */
>> int ovs_flow_key_extract_userspace(const struct nlattr *attr,
>> struct sk_buff *skb,
>> diff --git a/datapath/vport.c b/datapath/vport.c
>> index 5d250aa..2b218e8 100644
>> --- a/datapath/vport.c
>> +++ b/datapath/vport.c
>> @@ -475,6 +475,8 @@ void ovs_vport_receive(struct vport *vport, struct sk_buff *skb,
>> struct ovs_tunnel_info *tun_info)
>> {
>> struct pcpu_sw_netstats *stats;
>> + struct sw_flow_key key;
>> + int error;
>>
>> stats = this_cpu_ptr(vport->percpu_stats);
>> u64_stats_update_begin(&stats->syncp);
>> @@ -483,9 +485,14 @@ void ovs_vport_receive(struct vport *vport, struct sk_buff *skb,
>> u64_stats_update_end(&stats->syncp);
>>
>> ovs_skb_init_inner_protocol(skb);
>> - OVS_CB(skb)->tun_info = tun_info;
>> OVS_CB(skb)->input_vport = vport;
>> - ovs_dp_process_received_packet(skb);
>> + error = ovs_flow_key_extract(tun_info, skb, &key);
>> + if (unlikely(error)) {
>> + kfree_skb(skb);
>> + return;
>> + }
>> +
>> + ovs_dp_process_packet(skb, false);
>> }
>>
>> /**
>> --
>> 1.7.1
>>
>> _______________________________________________
>> dev mailing list
>> dev at openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev
More information about the dev
mailing list