| Message ID | 1452171912-29857-4-git-send-email-paul.durrant@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 2016-01-07 at 13:05 +0000, Paul Durrant wrote: > > /* > + * Hash types. (See NETIF_CTRL_TOEPLITZ_FLAG_* definitions above > + * for more information). > + */ > +#define XEN_NETIF_HASH_TYPE_NONE 0 > +#define XEN_NETIF_HASH_TYPE_IPV4 1 > +#define XEN_NETIF_HASH_TYPE_IPV4_TCP 2 > +#define XEN_NETIF_HASH_TYPE_IPV6 3 > +#define XEN_NETIF_HASH_TYPE_IPV6_TCP 4 Should these be TYPE_TOEPLITZ_FOO? I suppose there are other possible TCPv4 hashes for example. Perhaps a comment along the lines "XEN_NETIF_HASH_TYPE_TOEPLITZ_* corresponds precisely to the bit positions of NETIF_CTRL_TOEPLITZ_FLAG_* values", or even defining one in terms of the other? The control side (previous patch) has a toeplitz specific mechanism, with individual hash bits with it, whereas this is presented more as a generic hash mechanism with specific bits corresponding to toeplitz hashes. i.e. NETIF_CTRL_TOEPLITZ* vs XEN_NETIF_EXTRA_TYPE_HASH. Seems like a wrinkle, but I'm not sure if its an important one. Ian.
> -----Original Message----- > From: Ian Campbell [mailto:ian.campbell@citrix.com] > Sent: 08 January 2016 16:06 > To: Paul Durrant; xen-devel@lists.xenproject.org > Cc: Ian Jackson; Jan Beulich; Keir (Xen.org); Tim (Xen.org) > Subject: Re: [PATCH v4 3/3] public/io/netif.h: document new extra info for > passing hash values > > On Thu, 2016-01-07 at 13:05 +0000, Paul Durrant wrote: > > > > /* > > + * Hash types. (See NETIF_CTRL_TOEPLITZ_FLAG_* definitions above > > + * for more information). > > + */ > > +#define XEN_NETIF_HASH_TYPE_NONE 0 > > +#define XEN_NETIF_HASH_TYPE_IPV4 1 > > +#define XEN_NETIF_HASH_TYPE_IPV4_TCP 2 > > +#define XEN_NETIF_HASH_TYPE_IPV6 3 > > +#define XEN_NETIF_HASH_TYPE_IPV6_TCP 4 > > Should these be TYPE_TOEPLITZ_FOO? I suppose there are other possible > TCPv4 > hashes for example. I didn't consider that possibility of having multiple hashing schemes in operation at the same time, but I guess someone may want such a thing at some point. > > Perhaps a comment along the lines "XEN_NETIF_HASH_TYPE_TOEPLITZ_* > corresponds precisely to the bit positions of NETIF_CTRL_TOEPLITZ_FLAG_* > values", or even defining one in terms of the other? Yes, that sounds reasonable... > > The control side (previous patch) has a toeplitz specific mechanism, with > individual hash bits with it, whereas this is presented more as a generic > hash mechanism with specific bits corresponding to toeplitz hashes. > i.e. NETIF_CTRL_TOEPLITZ* vs XEN_NETIF_EXTRA_TYPE_HASH. Seems like a > wrinkle, but I'm not sure if its an important one. > ...if perhaps I make the extra type XEN_NETIF_EXTRA_TYPE_TOEPLITZ_HASH. That would leave things more open for someone else adding an extra type for another hash and needed a slightly different format/set of flags. Paul > Ian.
On Fri, 2016-01-08 at 16:26 +0000, Paul Durrant wrote: > > -----Original Message----- > > From: Ian Campbell [mailto:ian.campbell@citrix.com] > > Sent: 08 January 2016 16:06 > > To: Paul Durrant; xen-devel@lists.xenproject.org > > Cc: Ian Jackson; Jan Beulich; Keir (Xen.org); Tim (Xen.org) > > Subject: Re: [PATCH v4 3/3] public/io/netif.h: document new extra info > > for > > passing hash values > > > > On Thu, 2016-01-07 at 13:05 +0000, Paul Durrant wrote: > > > > > > /* > > > + * Hash types. (See NETIF_CTRL_TOEPLITZ_FLAG_* definitions above > > > + * for more information). > > > + */ > > > +#define XEN_NETIF_HASH_TYPE_NONE 0 > > > +#define XEN_NETIF_HASH_TYPE_IPV4 1 > > > +#define XEN_NETIF_HASH_TYPE_IPV4_TCP 2 > > > +#define XEN_NETIF_HASH_TYPE_IPV6 3 > > > +#define XEN_NETIF_HASH_TYPE_IPV6_TCP 4 > > > > Should these be TYPE_TOEPLITZ_FOO? I suppose there are other possible > > TCPv4 > > hashes for example. > > I didn't consider that possibility of having multiple hashing schemes in > operation at the same time, but I guess someone may want such a thing at > some point. Hrm, yes, I suppose IPV4_TCPa and IPV4_TCPb would be somewhat mutually exclusive. [...] > > > > The control side (previous patch) has a toeplitz specific mechanism, > > with > > individual hash bits with it, whereas this is presented more as a > > generic > > hash mechanism with specific bits corresponding to toeplitz hashes. > > i.e. NETIF_CTRL_TOEPLITZ* vs XEN_NETIF_EXTRA_TYPE_HASH. Seems like a > > wrinkle, but I'm not sure if its an important one. > > > > ...if perhaps I make the extra type XEN_NETIF_EXTRA_TYPE_TOEPLITZ_HASH. > That would leave things more open for someone else adding an extra type > for another hash and needed a slightly different format/set of flags. I'm happy with that (and think it's probably a good idea despite the comment above). Ian.
diff --git a/xen/include/public/io/netif.h b/xen/include/public/io/netif.h index 06e0b61..6c6bc9f 100644 --- a/xen/include/public/io/netif.h +++ b/xen/include/public/io/netif.h @@ -151,6 +151,12 @@ */ /* + * "feature-hash" advertises the capability to accept extra info slots of + * type XEN_NETIF_EXTRA_TYPE_HASH. They will not be sent by either end + * unless the other end advertises this feature. + */ + +/* * Control ring * ============ * @@ -574,6 +580,18 @@ DEFINE_RING_TYPES(netif_ctrl, struct netif_ctrl_request, struct netif_ctrl_respo * type: Must be XEN_NETIF_EXTRA_TYPE_MCAST_{ADD,DEL} * flags: XEN_NETIF_EXTRA_FLAG_* * addr: address to add/remove + * + * XEN_NETIF_EXTRA_TYPE_HASH: + * + * 0 1 2 3 4 5 6 7 octet + * +-----+-----+-----+-----+-----+-----+-----+-----+ + * |type |flags|htype| pad |LSB ---- value ---- MSB| + * +-----+-----+-----+-----+-----+-----+-----+-----+ + * + * type: Must be XEN_NETIF_EXTRA_TYPE_HASH + * flags: XEN_NETIF_EXTRA_FLAG_* + * htype: XEN_NETIF_HASH_TYPE_* + * value: Hash value */ /* Protocol checksum field is blank in the packet (hardware offload)? */ @@ -607,7 +625,8 @@ typedef struct netif_tx_request netif_tx_request_t; #define XEN_NETIF_EXTRA_TYPE_GSO (1) /* u.gso */ #define XEN_NETIF_EXTRA_TYPE_MCAST_ADD (2) /* u.mcast */ #define XEN_NETIF_EXTRA_TYPE_MCAST_DEL (3) /* u.mcast */ -#define XEN_NETIF_EXTRA_TYPE_MAX (4) +#define XEN_NETIF_EXTRA_TYPE_HASH (4) /* u.hash */ +#define XEN_NETIF_EXTRA_TYPE_MAX (5) /* netif_extra_info_t flags. */ #define _XEN_NETIF_EXTRA_FLAG_MORE (0) @@ -619,6 +638,16 @@ typedef struct netif_tx_request netif_tx_request_t; #define XEN_NETIF_GSO_TYPE_TCPV6 (2) /* + * Hash types. (See NETIF_CTRL_TOEPLITZ_FLAG_* definitions above + * for more information). + */ +#define XEN_NETIF_HASH_TYPE_NONE 0 +#define XEN_NETIF_HASH_TYPE_IPV4 1 +#define XEN_NETIF_HASH_TYPE_IPV4_TCP 2 +#define XEN_NETIF_HASH_TYPE_IPV6 3 +#define XEN_NETIF_HASH_TYPE_IPV6_TCP 4 + +/* * This structure needs to fit within both netif_tx_request_t and * netif_rx_response_t for compatibility. */ @@ -635,6 +664,11 @@ struct netif_extra_info { struct { uint8_t addr[6]; } mcast; + struct { + uint8_t type; + uint8_t pad; + uint8_t value[4]; + } hash; uint16_t pad[3]; } u; };
To properly support NDIS RSS, the Windows frontend PV driver needs the toeplitz hash value calculated by the backend (otherwise it would have to duplicate the calculation). This patch adds documentation for "feature-hash" and a definition of a new XEN_NETIF_EXTRA_TYPE_HASH extra info segment which both frontends and backends can use to pass hash values to the other end. Signed-off-by: Paul Durrant <paul.durrant@citrix.com> Cc: Ian Campbell <ian.campbell@citrix.com> Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Jan Beulich <jbeulich@suse.com> Cc: Keir Fraser <keir@xen.org> Cc: Tim Deegan <tim@xen.org> --- xen/include/public/io/netif.h | 36 +++++++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-)