[ovs-dev] [PATCH v2] datapath: Enable tunnel GSO features.

Pravin Shelar pshelar at nicira.com
Fri Jul 11 16:55:32 UTC 2014


On Thu, Jul 10, 2014 at 5:05 PM, Andy Zhou <azhou at nicira.com> wrote:
> The logic looks good.  Patchcheck returns some warnings that may be
> nice to fix up.
> Otherwise,  Acked-by: Andy Zhou <azhou at nicira.com>
>
>
> WARNING: LINUX_VERSION_CODE should be avoided, code should be for the
> version to which it is merged
> #29: FILE: datapath/linux/compat/include/linux/netdev_features.h:12:
> +#if LINUX_VERSION_CODE < KERNEL_VERSION(3,8,0)
>
> ERROR: space required after that ',' (ctx:VxV)
> #29: FILE: datapath/linux/compat/include/linux/netdev_features.h:12:
> +#if LINUX_VERSION_CODE < KERNEL_VERSION(3,8,0)
>                                           ^
>
> ERROR: space required after that ',' (ctx:VxV)
> #29: FILE: datapath/linux/compat/include/linux/netdev_features.h:12:
> +#if LINUX_VERSION_CODE < KERNEL_VERSION(3,8,0)
>                                             ^
>
> ERROR: code indent should use tabs where possible
> #104: FILE: datapath/linux/compat/include/net/vxlan.h:13:
> +                   struct rtable *rt, struct sk_buff *skb,$
>
> WARNING: please, no spaces at the start of a line
> #104: FILE: datapath/linux/compat/include/net/vxlan.h:13:
> +                   struct rtable *rt, struct sk_buff *skb,$
>
> ERROR: code indent should use tabs where possible
> #105: FILE: datapath/linux/compat/include/net/vxlan.h:14:
> +                   __be32 src, __be32 dst, __u8 tos, __u8 ttl, __be16 df,$
>
> WARNING: please, no spaces at the start of a line
> #105: FILE: datapath/linux/compat/include/net/vxlan.h:14:
> +                   __be32 src, __be32 dst, __u8 tos, __u8 ttl, __be16 df,$
>
> ERROR: code indent should use tabs where possible
> #106: FILE: datapath/linux/compat/include/net/vxlan.h:15:
> +                   __be16 src_port, __be16 dst_port, __be32 vni)$
>
> WARNING: please, no spaces at the start of a line
> #106: FILE: datapath/linux/compat/include/net/vxlan.h:15:
> +                   __be16 src_port, __be16 dst_port, __be32 vni)$
>
> WARNING: LINUX_VERSION_CODE should be avoided, code should be for the
> version to which it is merged
> #156: FILE: datapath/vport-internal_dev.c:168:
> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(3,8,0)
>
OVS compatibility needs this check. so this error is false positive for OVS.

> ERROR: space required after that ',' (ctx:VxV)
> #156: FILE: datapath/vport-internal_dev.c:168:
> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(3,8,0)
>                                            ^
This coding style is used since beginning, so I will keep it as it is.

I pushed this patch to master.

Thanks.

>
> ERROR: space required after that ',' (ctx:VxV)
> #156: FILE: datapath/vport-internal_dev.c:168:
> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(3,8,0)
>                                              ^
>
> total: 7 errors, 5 warnings, 127 lines checked
>
> NOTE: whitespace errors detected, you may wish to use scripts/cleanpatch or
>       scripts/cleanfile
>
> 0001-datapath-Enable-tunnel-GSO-features.patch has style problems,
> please review.
>
> If any of these errors are false positives, please report
> them to the maintainer, see CHECKPATCH in MAINTAINERS.
>
> On Thu, Jul 3, 2014 at 11:38 AM, Pravin B Shelar <pshelar at nicira.com> wrote:
>> Following patch enables all available tunnel GSO features for OVS
>> bridge device so that ovs can use hardware offloads available to
>> underling device.
>>
>> Signed-off-by: Pravin B Shelar <pshelar at nicira.com>
>> ---
>> v1-v2:
>> This patch checks for kernel version rather than supported
>> features.
>> ---
>>  .../linux/compat/include/linux/netdev_features.h   |   42 ++++++++++++++++++++
>>  datapath/linux/compat/include/net/gre.h            |   11 +++++
>>  datapath/linux/compat/include/net/vxlan.h          |   16 +++++++
>>  datapath/vport-geneve.c                            |    5 ++
>>  datapath/vport-internal_dev.c                      |    8 +++-
>>  datapath/vport-lisp.c                              |    5 ++
>>  6 files changed, 86 insertions(+), 1 deletions(-)
>>
>> diff --git a/datapath/linux/compat/include/linux/netdev_features.h b/datapath/linux/compat/include/linux/netdev_features.h
>> index 0259413..9f6331d 100644
>> --- a/datapath/linux/compat/include/linux/netdev_features.h
>> +++ b/datapath/linux/compat/include/linux/netdev_features.h
>> @@ -9,4 +9,46 @@
>>  #define NETIF_F_HW_VLAN_CTAG_TX NETIF_F_HW_VLAN_TX
>>  #endif
>>
>> +#if LINUX_VERSION_CODE < KERNEL_VERSION(3,8,0)
>> +#define NETIF_F_GSO_ENCAP_ALL   0
>> +
>> +#else
>> +
>> +#ifndef NETIF_F_GSO_GRE
>> +#define NETIF_F_GSO_GRE 0
>> +#endif
>> +
>> +#ifndef NETIF_F_GSO_GRE_CSUM
>> +#define NETIF_F_GSO_GRE_CSUM 0
>> +#endif
>> +
>> +#ifndef NETIF_F_GSO_IPIP
>> +#define NETIF_F_GSO_IPIP 0
>> +#endif
>> +
>> +#ifndef NETIF_F_GSO_SIT
>> +#define NETIF_F_GSO_SIT 0
>> +#endif
>> +
>> +#ifndef NETIF_F_GSO_UDP_TUNNEL
>> +#define NETIF_F_GSO_UDP_TUNNEL 0
>> +#endif
>> +
>> +#ifndef NETIF_F_GSO_UDP_TUNNEL_CSUM
>> +#define NETIF_F_GSO_UDP_TUNNEL_CSUM 0
>> +#endif
>> +
>> +#ifndef NETIF_F_GSO_MPLS
>> +#define NETIF_F_GSO_MPLS 0
>> +#endif
>> +
>> +#define NETIF_F_GSO_ENCAP_ALL  (NETIF_F_GSO_GRE |                      \
>> +                                NETIF_F_GSO_GRE_CSUM |                 \
>> +                                NETIF_F_GSO_IPIP |                     \
>> +                                NETIF_F_GSO_SIT |                      \
>> +                                NETIF_F_GSO_UDP_TUNNEL |               \
>> +                                NETIF_F_GSO_UDP_TUNNEL_CSUM |          \
>> +                                NETIF_F_GSO_MPLS)
>> +#endif
>> +
>>  #endif
>> diff --git a/datapath/linux/compat/include/net/gre.h b/datapath/linux/compat/include/net/gre.h
>> index dc03535..3c69e38 100644
>> --- a/datapath/linux/compat/include/net/gre.h
>> +++ b/datapath/linux/compat/include/net/gre.h
>> @@ -103,6 +103,17 @@ static inline int ip_gre_calc_hlen(__be16 o_flags)
>>                 addend += 4;
>>         return addend;
>>  }
>> +#else
>> +static inline struct sk_buff *rpl_gre_handle_offloads(struct sk_buff *skb,
>> +                                                 bool gre_csum)
>> +{
>> +       if (skb->encapsulation && skb_is_gso(skb)) {
>> +               kfree_skb(skb);
>> +               return ERR_PTR(-ENOSYS);
>> +       }
>> +       return gre_handle_offloads(skb, gre_csum);
>> +}
>> +#define gre_handle_offloads rpl_gre_handle_offloads
>>  #endif
>>
>>  #endif
>> diff --git a/datapath/linux/compat/include/net/vxlan.h b/datapath/linux/compat/include/net/vxlan.h
>> index 414a497..d64630b 100644
>> --- a/datapath/linux/compat/include/net/vxlan.h
>> +++ b/datapath/linux/compat/include/net/vxlan.h
>> @@ -8,6 +8,22 @@
>>  #include <linux/version.h>
>>  #if LINUX_VERSION_CODE >= KERNEL_VERSION(3,12,0)
>>  #include_next <net/vxlan.h>
>> +
>> +static inline int rpl_vxlan_xmit_skb(struct vxlan_sock *vs,
>> +                   struct rtable *rt, struct sk_buff *skb,
>> +                   __be32 src, __be32 dst, __u8 tos, __u8 ttl, __be16 df,
>> +                   __be16 src_port, __be16 dst_port, __be32 vni)
>> +{
>> +       if (skb->encapsulation && skb_is_gso(skb)) {
>> +               kfree_skb(skb);
>> +               return -ENOSYS;
>> +       }
>> +
>> +       return vxlan_xmit_skb(vs, rt, skb, src, dst, tos, ttl, df,
>> +                             src_port, dst_port, vni);
>> +}
>> +
>> +#define vxlan_xmit_skb rpl_vxlan_xmit_skb
>>  #else
>>
>>  struct vxlan_sock;
>> diff --git a/datapath/vport-geneve.c b/datapath/vport-geneve.c
>> index 33047f2..99841d4 100644
>> --- a/datapath/vport-geneve.c
>> +++ b/datapath/vport-geneve.c
>> @@ -333,6 +333,11 @@ static int handle_offloads(struct sk_buff *skb)
>>  #else
>>  static int handle_offloads(struct sk_buff *skb)
>>  {
>> +       if (skb->encapsulation && skb_is_gso(skb)) {
>> +               kfree_skb(skb);
>> +               return -ENOSYS;
>> +       }
>> +
>>         if (skb_is_gso(skb)) {
>>                 int err = skb_unclone(skb, GFP_ATOMIC);
>>                 if (unlikely(err))
>> diff --git a/datapath/vport-internal_dev.c b/datapath/vport-internal_dev.c
>> index 637d712..3b0f9a7 100644
>> --- a/datapath/vport-internal_dev.c
>> +++ b/datapath/vport-internal_dev.c
>> @@ -155,7 +155,8 @@ static void do_setup(struct net_device *netdev)
>>         netdev->tx_queue_len = 0;
>>
>>         netdev->features = NETIF_F_LLTX | NETIF_F_SG | NETIF_F_FRAGLIST |
>> -                          NETIF_F_HIGHDMA | NETIF_F_HW_CSUM | NETIF_F_GSO_SOFTWARE;
>> +                          NETIF_F_HIGHDMA | NETIF_F_HW_CSUM |
>> +                          NETIF_F_GSO_SOFTWARE | NETIF_F_GSO_ENCAP_ALL;
>>
>>         netdev->vlan_features = netdev->features;
>>         netdev->features |= NETIF_F_HW_VLAN_CTAG_TX;
>> @@ -163,6 +164,11 @@ static void do_setup(struct net_device *netdev)
>>  #if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,39)
>>         netdev->hw_features = netdev->features & ~NETIF_F_LLTX;
>>  #endif
>> +
>> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(3,8,0)
>> +       netdev->hw_enc_features = netdev->features;
>> +#endif
>> +
>>         eth_hw_addr_random(netdev);
>>  }
>>
>> diff --git a/datapath/vport-lisp.c b/datapath/vport-lisp.c
>> index c41e09e..81ecf92 100644
>> --- a/datapath/vport-lisp.c
>> +++ b/datapath/vport-lisp.c
>> @@ -409,6 +409,11 @@ static int handle_offloads(struct sk_buff *skb)
>>  #else
>>  static int handle_offloads(struct sk_buff *skb)
>>  {
>> +       if (skb->encapsulation && skb_is_gso(skb)) {
>> +               kfree_skb(skb);
>> +               return -ENOSYS;
>> +       }
>> +
>>         if (skb_is_gso(skb)) {
>>                 int err = skb_unclone(skb, GFP_ATOMIC);
>>                 if (unlikely(err))
>> --
>> 1.7.1
>>
>> _______________________________________________
>> dev mailing list
>> dev at openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev



More information about the dev mailing list