From a063b0dff0ccb6639f7e95969843d1c7cc2e15ae Mon Sep 17 00:00:00 2001 From: Jesse Gross Date: Sun, 28 Feb 2010 12:17:16 -0500 Subject: [PATCH] datapath: Consistently maintain checksum offloading state. When adding a VLAN tag it is necessary for us to setup checksum pointers for offloaded packets manually. However, this process clobbers some of the fields that other components need to determine the current status. Here we mark the packet with its status upon ingress in our own format that does not get clobbered and is consistent across kernel versions. Bug #2436 --- datapath/actions.c | 36 ++---------------------------------- datapath/datapath.c | 29 +++++++++++++++++++++++++++-- datapath/datapath.h | 17 +++++++++++++++++ 3 files changed, 46 insertions(+), 36 deletions(-) diff --git a/datapath/actions.c b/datapath/actions.c index b39d8307..b20873bf 100644 --- a/datapath/actions.c +++ b/datapath/actions.c @@ -215,42 +215,10 @@ static void update_csum(__sum16 *sum, struct sk_buff *skb, { __be32 diff[] = { ~from, to }; -/* On older kernels, CHECKSUM_PARTIAL and CHECKSUM_COMPLETE are both defined - * as CHECKSUM_HW. However, we can make some inferences so that we can update - * the checksums appropriately. */ - enum { - CSUM_PARTIAL, /* Partial checksum, skb->csum undefined. */ - CSUM_PACKET, /* In-packet checksum, skb->csum undefined. */ - CSUM_COMPLETE, /* In-packet checksum, skb->csum valid. */ - } csum_type; - - csum_type = CSUM_PACKET; -#ifndef CHECKSUM_HW - /* Newer kernel, just map between kernel types and ours. */ - if (skb->ip_summed == CHECKSUM_PARTIAL) - csum_type = CSUM_PARTIAL; - else if (skb->ip_summed == CHECKSUM_COMPLETE) - csum_type = CSUM_COMPLETE; -#else - /* In theory this could be either CHECKSUM_PARTIAL or CHECKSUM_COMPLETE. - * However, we should only get CHECKSUM_PARTIAL packets from Xen, which - * uses some special fields to represent this (see below). Since we - * can only make one type work, pick the one that actually happens in - * practice. */ - if (skb->ip_summed == CHECKSUM_HW) - csum_type = CSUM_COMPLETE; -#endif -#if defined(CONFIG_XEN) && defined(HAVE_PROTO_DATA_VALID) - /* Xen has a special way of representing CHECKSUM_PARTIAL on older - * kernels. */ - if (skb->proto_csum_blank) - csum_type = CSUM_PARTIAL; -#endif - - if (csum_type != CSUM_PARTIAL) { + if (OVS_CB(skb)->ip_summed != CSUM_PARTIAL) { *sum = csum_fold(csum_partial((char *)diff, sizeof(diff), ~csum_unfold(*sum))); - if (csum_type == CSUM_COMPLETE && pseudohdr) + if (OVS_CB(skb)->ip_summed == CSUM_COMPLETE && pseudohdr) skb->csum = ~csum_partial((char *)diff, sizeof(diff), ~skb->csum); } else if (pseudohdr) diff --git a/datapath/datapath.c b/datapath/datapath.c index 34d2c9bc..b6aefe89 100644 --- a/datapath/datapath.c +++ b/datapath/datapath.c @@ -71,6 +71,7 @@ static DEFINE_MUTEX(dp_mutex); #define MAINT_SLEEP_MSECS 1000 static int new_nbp(struct datapath *, struct net_device *, int port_no); +static void compute_ip_summed(struct sk_buff *skb); /* Must be called with rcu_read_lock or dp_mutex. */ struct datapath *get_dp(int dp_idx) @@ -529,6 +530,8 @@ void dp_process_received_packet(struct sk_buff *skb, struct net_bridge_port *p) WARN_ON_ONCE(skb_shared(skb)); + compute_ip_summed(skb); + /* BHs are off so we don't have to use get_cpu()/put_cpu() here. */ stats = percpu_ptr(dp->stats_percpu, smp_processor_id()); @@ -606,7 +609,7 @@ int vswitch_skb_checksum_setup(struct sk_buff *skb) if (skb->protocol != htons(ETH_P_IP)) goto out; - if (!skb_pull_up_to(skb, skb_network_header(skb) + 1)) + if (!skb_pull_up_to(skb, skb_network_header(skb) + sizeof(struct iphdr))) goto out; iph = ip_hdr(skb); @@ -696,11 +699,33 @@ out: * skb_forward_csum(). It is slightly different because we are only concerned * with bridging and not other types of forwarding and can get away with * slightly more optimal behavior.*/ +static void +compute_ip_summed(struct sk_buff *skb) +{ + OVS_CB(skb)->ip_summed = skb->ip_summed; + +#ifdef CHECKSUM_HW + /* In theory this could be either CHECKSUM_PARTIAL or CHECKSUM_COMPLETE. + * However, we should only get CHECKSUM_PARTIAL packets from Xen, which + * uses some special fields to represent this (see below). Since we + * can only make one type work, pick the one that actually happens in + * practice. */ + if (skb->ip_summed == CHECKSUM_HW) + OVS_CB(skb)->ip_summed = CSUM_COMPLETE; +#endif +#if defined(CONFIG_XEN) && defined(HAVE_PROTO_DATA_VALID) + /* Xen has a special way of representing CHECKSUM_PARTIAL on older + * kernels. */ + if (skb->proto_csum_blank) + OVS_CB(skb)->ip_summed = CSUM_PARTIAL; +#endif +} + void forward_ip_summed(struct sk_buff *skb) { #ifdef CHECKSUM_HW - if (skb->ip_summed == CHECKSUM_HW) + if (OVS_CB(skb)->ip_summed == CSUM_COMPLETE) skb->ip_summed = CHECKSUM_NONE; #endif } diff --git a/datapath/datapath.h b/datapath/datapath.h index 6732b59e..d9fe4b2f 100644 --- a/datapath/datapath.h +++ b/datapath/datapath.h @@ -182,6 +182,23 @@ struct net_bridge_port { atomic_t sflow_pool; }; +enum csum_type { + CSUM_NONE = 0, + CSUM_UNNECESSARY = 1, + CSUM_COMPLETE = 2, + CSUM_PARTIAL = 3, +}; + +/** + * struct ovs_skb_cb - OVS data in skb CB + * @ip_summed: Consistently stores L4 checksumming status across different + * kernel versions. + */ +struct ovs_skb_cb { + enum csum_type ip_summed; +}; +#define OVS_CB(skb) ((struct ovs_skb_cb *)(skb)->cb) + extern struct notifier_block dp_device_notifier; extern int (*dp_ioctl_hook)(struct net_device *dev, struct ifreq *rq, int cmd); -- 2.30.2