On Mon, May 4, 2020 at 9:02 AM Han Zhou <[email protected]> wrote: > > This patch backports below upstream patches, and add __skb_set_hash > to compat for older kernels. > > commit b5ab1f1be6180a2e975eede18731804b5164a05d > Author: Jakub Kicinski <[email protected]> > 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 <[email protected]> > Reviewed-by: Greg Rose <[email protected]> > Signed-off-by: David S. Miller <[email protected]> > > commit bd1903b7c4596ba6f7677d0dfefd05ba5876707d > Author: Tonghao Zhang <[email protected]> > 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 <[email protected]> > Acked-by: Pravin B Shelar <[email protected]> > Signed-off-by: David S. Miller <[email protected]> > > Cc: Tonghao Zhang <[email protected]> > Signed-off-by: Han Zhou <[email protected]> Reviewed-by: Tonghao Zhang <[email protected]> > --- > acinclude.m4 | 2 ++ > datapath/datapath.c | 27 ++++++++++++++++++++++++++- > datapath/datapath.h | 12 ++++++++++++ > datapath/linux/compat/include/linux/skbuff.h | 10 ++++++++++ > 4 files changed, 50 insertions(+), 1 deletion(-) > > diff --git a/acinclude.m4 b/acinclude.m4 > index dabbffd..5e5ca39 100644 > --- a/acinclude.m4 > +++ b/acinclude.m4 > @@ -1101,6 +1101,8 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [ > OVS_FIND_OP_PARAM_IFELSE([$KSRC/include/net/rtnetlink.h], > [validate], [extack], > [OVS_DEFINE([HAVE_RTNLOP_VALIDATE_WITH_EXTACK])]) > + OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], > + [__skb_set_hash]) > > if cmp -s datapath/linux/kcompat.h.new \ > datapath/linux/kcompat.h >/dev/null 2>&1; then > diff --git a/datapath/datapath.c b/datapath/datapath.c > index a7af784..82f5688 100644 > --- a/datapath/datapath.c > +++ b/datapath/datapath.c > @@ -371,7 +371,8 @@ static size_t upcall_msg_size(const struct dp_upcall_info > *upcall_info, > size_t size = NLMSG_ALIGN(sizeof(struct ovs_header)) > + nla_total_size(hdrlen) /* OVS_PACKET_ATTR_PACKET */ > + nla_total_size(ovs_key_attr_size()) /* OVS_PACKET_ATTR_KEY > */ > - + nla_total_size(sizeof(unsigned int)); /* > OVS_PACKET_ATTR_LEN */ > + + nla_total_size(sizeof(unsigned int)) /* OVS_PACKET_ATTR_LEN > */ > + + nla_total_size(sizeof(u64)); /* OVS_PACKET_ATTR_HASH */ > > /* OVS_PACKET_ATTR_USERDATA */ > if (upcall_info->userdata) > @@ -414,6 +415,7 @@ static int queue_userspace_packet(struct datapath *dp, > struct sk_buff *skb, > size_t len; > unsigned int hlen; > int err, dp_ifindex; > + u64 hash; > > dp_ifindex = get_dpifindex(dp); > if (!dp_ifindex) > @@ -523,6 +525,19 @@ static int queue_userspace_packet(struct datapath *dp, > struct sk_buff *skb, > pad_packet(dp, user_skb); > } > > + /* Add OVS_PACKET_ATTR_HASH */ > + hash = skb_get_hash_raw(skb); > + if (skb->sw_hash) > + hash |= OVS_PACKET_HASH_SW_BIT; > + > + if (skb->l4_hash) > + hash |= OVS_PACKET_HASH_L4_BIT; > + > + if (nla_put(user_skb, OVS_PACKET_ATTR_HASH, sizeof (u64), &hash)) { > + err = -ENOBUFS; > + goto out; > + } > + > /* Only reserve room for attribute header, packet data is added > * in skb_zerocopy() > */ > @@ -563,6 +578,7 @@ static int ovs_packet_cmd_execute(struct sk_buff *skb, > struct genl_info *info) > struct datapath *dp; > struct vport *input_vport; > u16 mru = 0; > + u64 hash; > int len; > int err; > bool log = !a[OVS_PACKET_ATTR_PROBE]; > @@ -588,6 +604,14 @@ static int ovs_packet_cmd_execute(struct sk_buff *skb, > struct genl_info *info) > } > OVS_CB(packet)->mru = mru; > > + if (a[OVS_PACKET_ATTR_HASH]) { > + hash = nla_get_u64(a[OVS_PACKET_ATTR_HASH]); > + > + __skb_set_hash(packet, hash & 0xFFFFFFFFULL, > + !!(hash & OVS_PACKET_HASH_SW_BIT), > + !!(hash & OVS_PACKET_HASH_L4_BIT)); > + } > + > /* Build an sw_flow for sending this packet. */ > flow = ovs_flow_alloc(); > err = PTR_ERR(flow); > @@ -649,6 +673,7 @@ static const struct nla_policy > packet_policy[OVS_PACKET_ATTR_MAX + 1] = { > [OVS_PACKET_ATTR_ACTIONS] = { .type = NLA_NESTED }, > [OVS_PACKET_ATTR_PROBE] = { .type = NLA_FLAG }, > [OVS_PACKET_ATTR_MRU] = { .type = NLA_U16 }, > + [OVS_PACKET_ATTR_HASH] = { .type = NLA_U64 }, > }; > > static struct genl_ops dp_packet_genl_ops[] = { > diff --git a/datapath/datapath.h b/datapath/datapath.h > index 3bffa1d..f99db1f 100644 > --- a/datapath/datapath.h > +++ b/datapath/datapath.h > @@ -159,6 +159,18 @@ struct ovs_net { > #endif > }; > > +/** > + * enum ovs_pkt_hash_types - hash info to include with a packet > + * to send to userspace. > + * @OVS_PACKET_HASH_SW_BIT: indicates hash was computed in software stack. > + * @OVS_PACKET_HASH_L4_BIT: indicates hash is a canonical 4-tuple hash > + * over transport ports. > + */ > +enum ovs_pkt_hash_types { > + OVS_PACKET_HASH_SW_BIT = (1ULL << 32), > + OVS_PACKET_HASH_L4_BIT = (1ULL << 33), > +}; > + > extern unsigned int ovs_net_id; > void ovs_lock(void); > void ovs_unlock(void); > diff --git a/datapath/linux/compat/include/linux/skbuff.h > b/datapath/linux/compat/include/linux/skbuff.h > index 6397289..930544e 100644 > --- a/datapath/linux/compat/include/linux/skbuff.h > +++ b/datapath/linux/compat/include/linux/skbuff.h > @@ -456,4 +456,14 @@ static inline void skb_set_inner_ipproto(struct sk_buff > *skb, > #define nf_reset_ct nf_reset > #endif > > +#ifndef HAVE___SKB_SET_HASH > +static inline void > +__skb_set_hash(struct sk_buff *skb, __u32 hash, bool is_sw, bool is_l4) > +{ > + skb->l4_hash = is_l4; > + skb->sw_hash = is_sw; > + skb->hash = hash; > +} > +#endif > + > #endif > -- > 2.1.0 >
-- Best regards, Tonghao _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
