[ovs-dev] [PATCH v2] datapath: Add hash info to upcall.

[Ilya Maximets]

On 5/27/20 8:56 AM, aginwala wrote:
> Thanks Han for backporting it for lower kernels. I tested this patch with some running workloads with old  3.13* kernels (ovs 2.11*) and it works fine. In addition I am also considering the discussion [1],  and for ecmp workloads I see all packets are hitting the different bucket instead of the same bucket (using ssh with single connection). Not a blocker as will plan to upgrade these nodes with 3.13*  to newer 4*+ kernels moving further as it's tough to leverage all ovs features for old kernels.
> 
> Tested-by: Aliasgar Ginwala <aginwala at ebay.com <mailto:aginwala at ebay.com>>
> 
> [1] - https://mail.openvswitch.org/pipermail/ovs-discuss/2020-April/049896.html
> 
> On Mon, May 25, 2020 at 11:28 PM Han Zhou <hzhou at ovn.org <mailto:hzhou at ovn.org>> wrote:
> 
>     This patch backports below upstream patches, and add __skb_set_hash
>     to compat for older kernels.
> 
>     commit b5ab1f1be6180a2e975eede18731804b5164a05d
>     Author: Jakub Kicinski <kuba at kernel.org <mailto:kuba at kernel.org>>
>     Date:   Mon Mar 2 21:05:18 2020 -0800
> 
>         openvswitch: add missing attribute validation for hash
> 
>         Add missing attribute validation for OVS_PACKET_ATTR_HASH
>         to the netlink policy.
> 
>         Fixes: bd1903b7c459 ("net: openvswitch: add hash info to upcall")
>         Signed-off-by: Jakub Kicinski <kuba at kernel.org <mailto:kuba at kernel.org>>
>         Reviewed-by: Greg Rose <gvrose8192 at gmail.com <mailto:gvrose8192 at gmail.com>>
>         Signed-off-by: David S. Miller <davem at davemloft.net <mailto:davem at davemloft.net>>
> 
>     commit bd1903b7c4596ba6f7677d0dfefd05ba5876707d
>     Author: Tonghao Zhang <xiangxia.m.yue at gmail.com <mailto:xiangxia.m.yue at gmail.com>>
>     Date:   Wed Nov 13 23:04:49 2019 +0800
> 
>         net: openvswitch: add hash info to upcall
> 
>         When using the kernel datapath, the upcall don't
>         include skb hash info relatived. That will introduce
>         some problem, because the hash of skb is important
>         in kernel stack. For example, VXLAN module uses
>         it to select UDP src port. The tx queue selection
>         may also use the hash in stack.
> 
>         Hash is computed in different ways. Hash is random
>         for a TCP socket, and hash may be computed in hardware,
>         or software stack. Recalculation hash is not easy.
> 
>         Hash of TCP socket is computed:
>         tcp_v4_connect
>             -> sk_set_txhash (is random)
> 
>         __tcp_transmit_skb
>             -> skb_set_hash_from_sk
> 
>         There will be one upcall, without information of skb
>         hash, to ovs-vswitchd, for the first packet of a TCP
>         session. The rest packets will be processed in Open vSwitch
>         modules, hash kept. If this tcp session is forward to
>         VXLAN module, then the UDP src port of first tcp packet
>         is different from rest packets.
> 
>         TCP packets may come from the host or dockers, to Open vSwitch.
>         To fix it, we store the hash info to upcall, and restore hash
>         when packets sent back.
> 
>         +---------------+          +-------------------------+
>         |   Docker/VMs  |          |     ovs-vswitchd        |
>         +----+----------+          +-+--------------------+--+
>              |                       ^                    |
>              |                       |                    |
>              |                       |  upcall            v restore packet hash
>     (not recalculate)
>              |                     +-+--------------------+--+
>              |  tap netdev         |                         |   vxlan module
>              +--------------->     +-->  Open vSwitch ko     +-->
>                or internal type    |                         |
>                                    +-------------------------+
> 
>         Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2019-October/364062.html
>         Signed-off-by: Tonghao Zhang <xiangxia.m.yue at gmail.com <mailto:xiangxia.m.yue at gmail.com>>
>         Acked-by: Pravin B Shelar <pshelar at ovn.org <mailto:pshelar at ovn.org>>
>         Signed-off-by: David S. Miller <davem at davemloft.net <mailto:davem at davemloft.net>>
> 
>     Acked-by: Tonghao Zhang <xiangxia.m.yue at gmail.com <mailto:xiangxia.m.yue at gmail.com>>
>     Signed-off-by: Han Zhou <hzhou at ovn.org <mailto:hzhou at ovn.org>>
>     ---
>     v1->v2: Fix build on kernel 3.16
> 
>      acinclude.m4                                 |  4 ++++
>      datapath/datapath.c                          | 33 +++++++++++++++++++++++++++-
>      datapath/datapath.h                          | 12 ++++++++++
>      datapath/linux/compat/include/linux/skbuff.h | 31 ++++++++++++++++++++++++++
>      4 files changed, 79 insertions(+), 1 deletion(-)
> 

Thanks!  Applied to master.

Best regards, Ilya Maximets.