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

[Ilya Maximets]

On 5/24/20 8:41 PM, Han Zhou wrote:
> 
> 
> On Thu, May 7, 2020 at 11:21 PM Tonghao Zhang <xiangxia.m.yue at gmail.com <mailto:xiangxia.m.yue at gmail.com>> wrote:
>>
>> On Mon, May 4, 2020 at 9:02 AM 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>>
>> >
>> > Cc: 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>>
>> Reviewed-by: Tonghao Zhang <xiangxia.m.yue at gmail.com <mailto:xiangxia.m.yue at gmail.com>>
> 
> Thanks review from Tonghao and Greg.
> It has been 3 weeks, so cc some maintainers and pop it up.


Hi.  Thanks for working on this!
There is an issue while building with 3.16 kernel:

ovs/datapath/linux/compat/include/linux/skbuff.h: In function ‘__skb_set_hash’:
ovs/datapath/linux/compat/include/linux/skbuff.h:464:5: error: ‘struct sk_buff’ has no member named ‘sw_hash’
  skb->sw_hash = is_sw;
     ^

https://travis-ci.org/github/igsilya/ovs/jobs/690987401

Best regards, Ilya Maximets.