| Message ID | 1468808649-24552-8-git-send-email-pshelar@ovn.org |
|---|---|
| State | Changes Requested |
| Headers | show |
On Mon, Jul 18, 2016 at 4:24 AM, Pravin B Shelar <pshelar@ovn.org> wrote: > diff --git a/datapath/linux/compat/include/linux/if_ether.h b/datapath/linux/compat/include/linux/if_ether.h > index b2cb56d..ac0f1ed 100644 > --- a/datapath/linux/compat/include/linux/if_ether.h > +++ b/datapath/linux/compat/include/linux/if_ether.h > @@ -11,10 +11,9 @@ > #define ETH_P_8021AD 0x88A8 /* 802.1ad Service VLAN */ > #endif > > -#ifndef HAVE_INNER_ETH_HDR > +#define inner_eth_hdr rpl_inner_eth_hdr > static inline struct ethhdr *inner_eth_hdr(const struct sk_buff *skb) > { > return (struct ethhdr *)skb_inner_mac_header(skb); > } > #endif > -#endif Why do we need this change? It looks like inner_eth_hdr() and skb_inner_mac_header() are the same on all kernel versions that we support. > diff --git a/datapath/linux/compat/include/net/udp_tunnel.h b/datapath/linux/compat/include/net/udp_tunnel.h > index ded7f30..5ba1cbf 100644 > --- a/datapath/linux/compat/include/net/udp_tunnel.h > +++ b/datapath/linux/compat/include/net/udp_tunnel.h [...] > static inline int rpl_udp_tunnel_handle_offloads(struct sk_buff *skb, > - bool udp_csum, > - bool is_vxlan) > + bool udp_csum) > { > int type = 0; > > @@ -178,14 +164,8 @@ static inline int rpl_udp_tunnel_handle_offloads(struct sk_buff *skb, > else > fix_segment = ovs_udp_csum_gso; > > -#if LINUX_VERSION_CODE < KERNEL_VERSION(3,18,0) > - if (!is_vxlan) > - type = 0; > -#endif I don't know that it is safe to remove this. The intention was to avoid problems that assumed UDP_TUNNEL offloads really meant VXLAN on kernels where VXLAN was the only UDP tunnel. > diff --git a/datapath/linux/compat/lisp.c b/datapath/linux/compat/lisp.c > index dd23059..c9cc809 100644 > --- a/datapath/linux/compat/lisp.c > +++ b/datapath/linux/compat/lisp.c > @@ -290,6 +290,29 @@ static struct rtable *lisp_get_rt(struct sk_buff *skb, > return ip_route_output_key(net, fl); > } > > +/* this is to handle the return type change in handle-offload > + * functions. > + */ > +#if !defined(HAVE_UDP_TUNNEL_HANDLE_OFFLOAD_RET_SKB) || !defined(USE_UPSTREAM_TUNNEL) > +static struct sk_buff * > +__udp_tunnel_handle_offloads(struct sk_buff *skb, bool udp_csum) > +{ > + int err; > + > + if (skb_is_gso(skb) && skb_is_encapsulated(skb)) { > + return ERR_PTR(-ENOSYS); > + } > + err = udp_tunnel_handle_offloads(skb, udp_csum); > + if (err) { > + kfree_skb(skb); > + return NULL; > + } > + return skb; > +} > +#else > +#define __udp_tunnel_handle_offloads udp_tunnel_handle_offloads > +#endif Aren't the checks in __udp_tunnel_handle_offloads() mostly about the fact that we don't have a device layer to deal with offloads? In that case, I think they would still be needed in all situations with LISP/STT.
On Mon, Jul 18, 2016 at 11:51 AM, Jesse Gross <jesse@kernel.org> wrote: > On Mon, Jul 18, 2016 at 4:24 AM, Pravin B Shelar <pshelar@ovn.org> wrote: >> diff --git a/datapath/linux/compat/include/linux/if_ether.h b/datapath/linux/compat/include/linux/if_ether.h >> index b2cb56d..ac0f1ed 100644 >> --- a/datapath/linux/compat/include/linux/if_ether.h >> +++ b/datapath/linux/compat/include/linux/if_ether.h >> @@ -11,10 +11,9 @@ >> #define ETH_P_8021AD 0x88A8 /* 802.1ad Service VLAN */ >> #endif >> >> -#ifndef HAVE_INNER_ETH_HDR >> +#define inner_eth_hdr rpl_inner_eth_hdr >> static inline struct ethhdr *inner_eth_hdr(const struct sk_buff *skb) >> { >> return (struct ethhdr *)skb_inner_mac_header(skb); >> } >> #endif >> -#endif > > Why do we need this change? It looks like inner_eth_hdr() and > skb_inner_mac_header() are the same on all kernel versions that we > support. > inner_eth_hdr is defined for 4.6 kernel. I have to back port it. Since it is really simple function I decided to avoid the configure grep and just rename it. >> diff --git a/datapath/linux/compat/include/net/udp_tunnel.h b/datapath/linux/compat/include/net/udp_tunnel.h >> index ded7f30..5ba1cbf 100644 >> --- a/datapath/linux/compat/include/net/udp_tunnel.h >> +++ b/datapath/linux/compat/include/net/udp_tunnel.h > [...] >> static inline int rpl_udp_tunnel_handle_offloads(struct sk_buff *skb, >> - bool udp_csum, >> - bool is_vxlan) >> + bool udp_csum) >> { >> int type = 0; >> >> @@ -178,14 +164,8 @@ static inline int rpl_udp_tunnel_handle_offloads(struct sk_buff *skb, >> else >> fix_segment = ovs_udp_csum_gso; >> >> -#if LINUX_VERSION_CODE < KERNEL_VERSION(3,18,0) >> - if (!is_vxlan) >> - type = 0; >> -#endif > > I don't know that it is safe to remove this. The intention was to > avoid problems that assumed UDP_TUNNEL offloads really meant VXLAN on > kernels where VXLAN was the only UDP tunnel. > right, I missed it. At present this function is not used by vxlan, so type reset can done unconditionally. >> diff --git a/datapath/linux/compat/lisp.c b/datapath/linux/compat/lisp.c >> index dd23059..c9cc809 100644 >> --- a/datapath/linux/compat/lisp.c >> +++ b/datapath/linux/compat/lisp.c >> @@ -290,6 +290,29 @@ static struct rtable *lisp_get_rt(struct sk_buff *skb, >> return ip_route_output_key(net, fl); >> } >> >> +/* this is to handle the return type change in handle-offload >> + * functions. >> + */ >> +#if !defined(HAVE_UDP_TUNNEL_HANDLE_OFFLOAD_RET_SKB) || !defined(USE_UPSTREAM_TUNNEL) >> +static struct sk_buff * >> +__udp_tunnel_handle_offloads(struct sk_buff *skb, bool udp_csum) >> +{ >> + int err; >> + >> + if (skb_is_gso(skb) && skb_is_encapsulated(skb)) { >> + return ERR_PTR(-ENOSYS); >> + } >> + err = udp_tunnel_handle_offloads(skb, udp_csum); >> + if (err) { >> + kfree_skb(skb); >> + return NULL; >> + } >> + return skb; >> +} >> +#else >> +#define __udp_tunnel_handle_offloads udp_tunnel_handle_offloads >> +#endif > > Aren't the checks in __udp_tunnel_handle_offloads() mostly about the > fact that we don't have a device layer to deal with offloads? In that > case, I think they would still be needed in all situations with > LISP/STT. right, it should be done for lisp. For STT I will send separate patch.
diff --git a/acinclude.m4 b/acinclude.m4 index 6b8d30f..641946b 100644 --- a/acinclude.m4 +++ b/acinclude.m4 @@ -134,10 +134,10 @@ AC_DEFUN([OVS_CHECK_LINUX], [ AC_MSG_RESULT([$kversion]) if test "$version" -ge 4; then - if test "$version" = 4 && test "$patchlevel" -le 5; then + if test "$version" = 4 && test "$patchlevel" -le 6; then : # Linux 4.x else - AC_ERROR([Linux kernel in $KBUILD is version $kversion, but version newer than 4.5.x is not supported (please refer to the FAQ for advice)]) + AC_ERROR([Linux kernel in $KBUILD is version $kversion, but version newer than 4.6.x is not supported (please refer to the FAQ for advice)]) fi elif test "$version" = 3 && test "$patchlevel" -ge 10; then : # Linux 3.x @@ -393,7 +393,6 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [ OVS_GREP_IFELSE([$KSRC/include/linux/etherdevice.h], [eth_hw_addr_random]) OVS_GREP_IFELSE([$KSRC/include/linux/etherdevice.h], [ether_addr_copy]) - OVS_GREP_IFELSE([$KSRC/nclude/linux/if_ether.h], [inner_eth_hdr]) OVS_GREP_IFELSE([$KSRC/include/uapi/linux/if_link.h], [IFLA_GENEVE_TOS]) OVS_GREP_IFELSE([$KSRC/include/uapi/linux/if_link.h], [rtnl_link_stats64]) @@ -642,6 +641,8 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [ OVS_GREP_IFELSE([$KSRC/include/net/udp.h], [udp_v4_check]) OVS_GREP_IFELSE([$KSRC/include/net/udp.h], [udp_set_csum]) OVS_GREP_IFELSE([$KSRC/include/net/udp_tunnel.h], [udp_tunnel_gro_complete]) + OVS_GREP_IFELSE([$KSRC/include/net/udp_tunnel.h], [sk_buff.*udp_tunnel_handle_offloads], + [OVS_DEFINE([HAVE_UDP_TUNNEL_HANDLE_OFFLOAD_RET_SKB])]) OVS_FIND_FIELD_IFELSE([$KSRC/include/net/udp_tunnel.h], [udp_tunnel_sock_cfg], [gro_receive]) diff --git a/datapath/linux/compat/geneve.c b/datapath/linux/compat/geneve.c index 1d5b291..f9e64d5 100644 --- a/datapath/linux/compat/geneve.c +++ b/datapath/linux/compat/geneve.c @@ -757,7 +757,7 @@ static int geneve_build_skb(struct rtable *rt, struct sk_buff *skb, if (unlikely(err)) goto free_rt; - err = udp_tunnel_handle_offloads(skb, udp_sum, false); + err = udp_tunnel_handle_offloads(skb, udp_sum); if (err) goto free_rt; @@ -790,7 +790,7 @@ static int geneve6_build_skb(struct dst_entry *dst, struct sk_buff *skb, if (unlikely(err)) goto free_dst; - err = udp_tunnel_handle_offloads(skb, udp_sum, false); + err = udp_tunnel_handle_offloads(skb, udp_sum); if (err) goto free_dst; diff --git a/datapath/linux/compat/include/linux/if_ether.h b/datapath/linux/compat/include/linux/if_ether.h index b2cb56d..ac0f1ed 100644 --- a/datapath/linux/compat/include/linux/if_ether.h +++ b/datapath/linux/compat/include/linux/if_ether.h @@ -11,10 +11,9 @@ #define ETH_P_8021AD 0x88A8 /* 802.1ad Service VLAN */ #endif -#ifndef HAVE_INNER_ETH_HDR +#define inner_eth_hdr rpl_inner_eth_hdr static inline struct ethhdr *inner_eth_hdr(const struct sk_buff *skb) { return (struct ethhdr *)skb_inner_mac_header(skb); } #endif -#endif diff --git a/datapath/linux/compat/include/net/dst_cache.h b/datapath/linux/compat/include/net/dst_cache.h index 53ca42a..ff4d83b 100644 --- a/datapath/linux/compat/include/net/dst_cache.h +++ b/datapath/linux/compat/include/net/dst_cache.h @@ -7,7 +7,10 @@ #include <net/ip6_fib.h> #endif -#ifndef USE_UPSTREAM_TUNNEL +#ifdef USE_UPSTREAM_TUNNEL +#include_next <net/dst_cache.h> + +#else struct dst_cache { struct dst_cache_pcpu __percpu *cache; unsigned long reset_ts; diff --git a/datapath/linux/compat/include/net/dst_metadata.h b/datapath/linux/compat/include/net/dst_metadata.h index 6660dfc..279b714 100644 --- a/datapath/linux/compat/include/net/dst_metadata.h +++ b/datapath/linux/compat/include/net/dst_metadata.h @@ -48,7 +48,6 @@ static inline struct metadata_dst *metadata_dst_alloc(u8 optslen, gfp_t flags) } #define skb_tunnel_info ovs_skb_tunnel_info -#endif static inline void ovs_tun_rx_dst(struct metadata_dst *md_dst, int optslen) { @@ -100,6 +99,8 @@ static inline void ovs_ipv6_tun_rx_dst(struct metadata_dst *md_dst, info->key.label = ip6_flowlabel(ip6h); } +#endif /* USE_UPSTREAM_TUNNEL */ + void ovs_ip_tunnel_rcv(struct net_device *dev, struct sk_buff *skb, struct metadata_dst *tun_dst); #endif /* __NET_DST_METADATA_WRAPPER_H */ diff --git a/datapath/linux/compat/include/net/ip_tunnels.h b/datapath/linux/compat/include/net/ip_tunnels.h index 6775c9a..3df770d 100644 --- a/datapath/linux/compat/include/net/ip_tunnels.h +++ b/datapath/linux/compat/include/net/ip_tunnels.h @@ -91,9 +91,6 @@ struct tnl_ptk_info { #undef TUNNEL_OPTIONS_PRESENT #define TUNNEL_OPTIONS_PRESENT (TUNNEL_GENEVE_OPT | TUNNEL_VXLAN_OPT) -#define skb_is_encapsulated ovs_skb_is_encapsulated -bool ovs_skb_is_encapsulated(struct sk_buff *skb); - /* Used to memset ip_tunnel padding. */ #define IP_TUNNEL_KEY_SIZE offsetofend(struct ip_tunnel_key, tp_dst) @@ -345,4 +342,8 @@ static inline int iptunnel_pull_offloads(struct sk_buff *skb) return 0; } #endif /* USE_UPSTREAM_TUNNEL */ + +#define skb_is_encapsulated ovs_skb_is_encapsulated +bool ovs_skb_is_encapsulated(struct sk_buff *skb); + #endif /* __NET_IP_TUNNELS_H */ diff --git a/datapath/linux/compat/include/net/udp_tunnel.h b/datapath/linux/compat/include/net/udp_tunnel.h index ded7f30..5ba1cbf 100644 --- a/datapath/linux/compat/include/net/udp_tunnel.h +++ b/datapath/linux/compat/include/net/udp_tunnel.h @@ -11,19 +11,6 @@ #ifdef USE_UPSTREAM_TUNNEL #include_next <net/udp_tunnel.h> -/* this is to handle the return type change in handle-offload - * functions. - */ -static inline int -rpl_udp_tunnel_handle_offloads(struct sk_buff *skb, bool udp_csum, - bool is_vxlan) -{ - if (skb_is_gso(skb) && skb_is_encapsulated(skb)) { - return -ENOSYS; - } - return udp_tunnel_handle_offloads(skb, udp_csum); -} - #else #include <net/addrconf.h> @@ -161,8 +148,7 @@ void ovs_udp_gso(struct sk_buff *skb); void ovs_udp_csum_gso(struct sk_buff *skb); static inline int rpl_udp_tunnel_handle_offloads(struct sk_buff *skb, - bool udp_csum, - bool is_vxlan) + bool udp_csum) { int type = 0; @@ -178,14 +164,8 @@ static inline int rpl_udp_tunnel_handle_offloads(struct sk_buff *skb, else fix_segment = ovs_udp_csum_gso; -#if LINUX_VERSION_CODE < KERNEL_VERSION(3,18,0) - if (!is_vxlan) - type = 0; -#endif - return ovs_iptunnel_handle_offloads(skb, udp_csum, type, fix_segment); } -#endif /* USE_UPSTREAM_TUNNEL */ #define udp_tunnel_handle_offloads rpl_udp_tunnel_handle_offloads static inline void ovs_udp_tun_rx_dst(struct metadata_dst *md_dst, @@ -205,5 +185,6 @@ static inline void ovs_udp_tun_rx_dst(struct metadata_dst *md_dst, if (udp_hdr(skb)->check) info->key.tun_flags |= TUNNEL_CSUM; } +#endif /* USE_UPSTREAM_TUNNEL */ #endif diff --git a/datapath/linux/compat/lisp.c b/datapath/linux/compat/lisp.c index dd23059..c9cc809 100644 --- a/datapath/linux/compat/lisp.c +++ b/datapath/linux/compat/lisp.c @@ -290,6 +290,29 @@ static struct rtable *lisp_get_rt(struct sk_buff *skb, return ip_route_output_key(net, fl); } +/* this is to handle the return type change in handle-offload + * functions. + */ +#if !defined(HAVE_UDP_TUNNEL_HANDLE_OFFLOAD_RET_SKB) || !defined(USE_UPSTREAM_TUNNEL) +static struct sk_buff * +__udp_tunnel_handle_offloads(struct sk_buff *skb, bool udp_csum) +{ + int err; + + if (skb_is_gso(skb) && skb_is_encapsulated(skb)) { + return ERR_PTR(-ENOSYS); + } + err = udp_tunnel_handle_offloads(skb, udp_csum); + if (err) { + kfree_skb(skb); + return NULL; + } + return skb; +} +#else +#define __udp_tunnel_handle_offloads udp_tunnel_handle_offloads +#endif + netdev_tx_t rpl_lisp_xmit(struct sk_buff *skb) { struct net_device *dev = skb->dev; @@ -344,9 +367,9 @@ netdev_tx_t rpl_lisp_xmit(struct sk_buff *skb) skb_reset_mac_header(skb); skb->vlan_tci = 0; - err = udp_tunnel_handle_offloads(skb, false, false); - if (err) - goto err_free_rt; + skb = __udp_tunnel_handle_offloads(skb, false); + if (!skb) + return NETDEV_TX_OK; src_port = htons(get_src_port(net, skb)); dst_port = lisp_dev->dst_port;
Most of patch iron out USE_UPSTREAM_TUNNEL case where datapath directly use upstream tunneling modules. Signed-off-by: Pravin B Shelar <pshelar@ovn.org> --- acinclude.m4 | 7 +++--- datapath/linux/compat/geneve.c | 4 ++-- datapath/linux/compat/include/linux/if_ether.h | 3 +-- datapath/linux/compat/include/net/dst_cache.h | 5 +++- datapath/linux/compat/include/net/dst_metadata.h | 3 ++- datapath/linux/compat/include/net/ip_tunnels.h | 7 +++--- datapath/linux/compat/include/net/udp_tunnel.h | 23 ++----------------- datapath/linux/compat/lisp.c | 29 +++++++++++++++++++++--- 8 files changed, 45 insertions(+), 36 deletions(-)