| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20140618081855.GC14968@verge.net.au>
Date: Wed, 18 Jun 2014 17:18:56 +0900
From: Simon Horman <horms@...ge.net.au>
To: Jesse Gross <jesse@...ira.com>
Cc: "dev@...nvswitch.org" <dev@...nvswitch.org>,
netdev <netdev@...r.kernel.org>, Ravi K <rkerur@...il.com>,
Joe Stringer <joe@...d.net.nz>, Thomas Graf <tgraf@...g.ch>
Subject: Re: [PATCH v2.60] datapath: Add basic MPLS support to kernel
On Tue, Jun 17, 2014 at 11:02:21PM -0700, Jesse Gross wrote:
> I'm currently seeing this compiler error:
>
> CC [M] /home/jesse/openvswitch/datapath/linux/gso.o
> /home/jesse/openvswitch/datapath/linux/gso.c: In function ‘tnl_skb_gso_segment’:
> /home/jesse/openvswitch/datapath/linux/gso.c:199:2: error: implicit
> declaration of function ‘skb_inner_mac_offset’
> [-Werror=implicit-function-declaration]
> int mac_offset = skb_inner_mac_offset(skb);
> ^
> /home/jesse/openvswitch/datapath/linux/gso.c:233:3: error: implicit
> declaration of function ‘OVS_GSO_CB’
> [-Werror=implicit-function-declaration]
> if (OVS_GSO_CB(skb)->fix_segment)
> ^
> /home/jesse/openvswitch/datapath/linux/gso.c:233:22: error: invalid
> type argument of ‘->’ (have ‘int’)
> if (OVS_GSO_CB(skb)->fix_segment)
> ^
> /home/jesse/openvswitch/datapath/linux/gso.c:234:19: error: invalid
> type argument of ‘->’ (have ‘int’)
> OVS_GSO_CB(skb)->fix_segment(skb);
>
> This is on 3.13. I was originally planning on trying to fix this
> myself but I'm obviously just slowing things down at this point :)
>
> This is a consequence of the recent extension of the versions that the
> compat code is covering.
I came up with two ways to resolve this problem.
1. Tweak the KERNEL_VERSION(3,12,0) line to KERNEL_VERSION(3,16,0)
near the top of datapath/linux/compat/gso.h and update the comment
a bit further down accordingly.
I think this should be correct but it seems a but dirty
to have struct ovs_gso_cb when it isn't really needed any more.
2. The following change which avoids using struct ovs_gso_cb after
v3.12 and allows using skb_inner_mac_offset up to v3.16.
This is my preferred approach.
diff --git a/datapath/linux/compat/gso.c b/datapath/linux/compat/gso.c
index dc1e537..8344293 100644
--- a/datapath/linux/compat/gso.c
+++ b/datapath/linux/compat/gso.c
@@ -190,6 +190,16 @@ static __be16 __skb_network_protocol(struct sk_buff *skb)
return type;
}
+#if LINUX_VERSION_CODE < KERNEL_VERSION(3,12,0)
+static void tnl_fix_segment(struct sk_buff *skb)
+{
+ if (OVS_GSO_CB(skb)->fix_segment)
+ OVS_GSO_CB(skb)->fix_segment(skb);
+}
+#else
+static void tnl_fix_segment(struct sk_buff *skb) { }
+#endif
+
static struct sk_buff *tnl_skb_gso_segment(struct sk_buff *skb,
netdev_features_t features,
bool tx_path)
@@ -230,8 +240,7 @@ static struct sk_buff *tnl_skb_gso_segment(struct sk_buff *skb,
memcpy(ip_hdr(skb), iph, pkt_hlen);
memcpy(skb->cb, cb, sizeof(cb));
- if (OVS_GSO_CB(skb)->fix_segment)
- OVS_GSO_CB(skb)->fix_segment(skb);
+ tnl_fix_segment(skb);
skb->protocol = proto;
skb = skb->next;
diff --git a/datapath/linux/compat/gso.h b/datapath/linux/compat/gso.h
index 1393173..b08ad41 100644
--- a/datapath/linux/compat/gso.h
+++ b/datapath/linux/compat/gso.h
@@ -54,12 +54,6 @@ static inline int skb_inner_network_offset(const struct sk_buff *skb)
return skb_inner_network_header(skb) - skb->data;
}
-#define skb_inner_mac_offset rpl_skb_inner_mac_offset
-static inline int skb_inner_mac_offset(const struct sk_buff *skb)
-{
- return skb_inner_mac_header(skb) - skb->data;
-}
-
#define skb_reset_inner_headers rpl_skb_reset_inner_headers
static inline void skb_reset_inner_headers(struct sk_buff *skb)
{
@@ -76,6 +70,14 @@ int ip_local_out(struct sk_buff *skb);
#endif /* 3.12 */
+#if LINUX_VERSION_CODE < KERNEL_VERSION(3,16,0)
+#define skb_inner_mac_offset rpl_skb_inner_mac_offset
+static inline int skb_inner_mac_offset(const struct sk_buff *skb)
+{
+ return skb_inner_mac_header(skb) - skb->data;
+}
+#endif
+
#if LINUX_VERSION_CODE < KERNEL_VERSION(3,11,0)
static inline void ovs_skb_init_inner_protocol(struct sk_buff *skb) {
OVS_GSO_CB(skb)->inner_protocol = htons(0);
I also ran into a new compile problem when rebasing. My proposed solution
is as follows:
diff --git a/datapath/datapath.c b/datapath/datapath.c
index 64041ff..73c215b 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -382,7 +382,7 @@ static size_t key_attr_size(void)
{
/* Whenever adding new OVS_KEY_ FIELDS, we should consider
* updating this function. */
- BUILD_BUG_ON(OVS_KEY_ATTR_IPV4_TUNNEL != 21);
+ BUILD_BUG_ON(OVS_KEY_ATTR_IPV4_TUNNEL != 22);
return nla_total_size(4) /* OVS_KEY_ATTR_PRIORITY */
+ nla_total_size(0) /* OVS_KEY_ATTR_TUNNEL */
@@ -397,6 +397,7 @@ static size_t key_attr_size(void)
+ nla_total_size(4) /* OVS_KEY_ATTR_SKB_MARK */
+ nla_total_size(4) /* OVS_KEY_ATTR_DP_HASH */
+ nla_total_size(4) /* OVS_KEY_ATTR_RECIRC_ID */
+ + nla_total_size(4) /* OVS_KEY_ATTR_MPLS */
+ nla_total_size(12) /* OVS_KEY_ATTR_ETHERNET */
+ nla_total_size(2) /* OVS_KEY_ATTR_ETHERTYPE */
+ nla_total_size(4) /* OVS_KEY_ATTR_8021Q */
> One thing that comes to mind is how do have
> correct behavior but avoid forcing unnecessary software segmentation
> for tunnels on later kernels.
An interesting problem that I had not considered.
Is the problem related to the presence of MPLS or the fact
that the compatibility code is now used in newer kernels?
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists