[ovs-dev] [PATCH 3/3] datapath: Use tun_info only for egress tunnel path.
Andy Zhou
azhou at nicira.com
Wed Aug 6 22:30:44 UTC 2014
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.
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?
>
> 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?
> + 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