[PATCH net-next] net: Reflect RX hash for transmit

[PATCH net-next] net: Reflect RX hash for transmit

[Tom Herbert]

This patch allows a driver to request that the receive hash it provides
to the stack is reflected back in skb->hash for packets transmitted
on the associated connection. The hash value returned by a device could
have meaning when used in transmit to identify a flow, for instance
the hash may by a flow ID for a flow created in a (virtual) device.
With the flow ID provided on transmit this can obviate the need
to create the hash and do the lookup on the fly in transmit.

Note that this is an opportunistic optimization, there is no
separate negotiation between the stack and driver/device for
the hash used in either TX or RX.

Signed-off-by: Tom Herbert <therbert@google.com>
---
 include/linux/skbuff.h | 3 ++-
 include/net/sock.h     | 2 ++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 1bb36ed..f792a0d 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -605,7 +605,8 @@ struct sk_buff {
 	__u8			ipvs_property:1;
 	__u8			inner_protocol_type:1;
 	__u8			remcsum_offload:1;
-	/* 3 or 5 bit hole */
+	__u8			reflect_hash:1;
+	/* 2 or 4 bit hole */
 
 #ifdef CONFIG_NET_SCHED
 	__u16			tc_index;	/* traffic control index */
diff --git a/include/net/sock.h b/include/net/sock.h
index e138245..133fc51 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -871,6 +871,8 @@ static inline void sock_rps_save_rxhash(struct sock *sk,
 	if (unlikely(sk->sk_rxhash != skb->hash))
 		sk->sk_rxhash = skb->hash;
 #endif
+	if (unlikely(skb->reflect_hash && sk->sk_txhash != skb->hash))
+		sk->sk_txhash = skb->hash;
 }
 
 static inline void sock_rps_reset_rxhash(struct sock *sk)
-- 
2.2.0.rc0.207.ga3a616c

Re: [PATCH net-next] net: Reflect RX hash for transmit

[Daniel Borkmann]

On 02/15/2015 07:37 PM, Tom Herbert wrote:
> This patch allows a driver to request that the receive hash it provides
> to the stack is reflected back in skb->hash for packets transmitted
> on the associated connection.
[...]
> +	if (unlikely(skb->reflect_hash && sk->sk_txhash != skb->hash))
> +		sk->sk_txhash = skb->hash;

Is there also an initial user setting this somewhere?

Re: [PATCH net-next] net: Reflect RX hash for transmit

[Tom Herbert]

On Sun, Feb 15, 2015 at 11:59 AM, Daniel Borkmann <daniel@iogearbox.net> wrote:
> On 02/15/2015 07:37 PM, Tom Herbert wrote:
>>
>> This patch allows a driver to request that the receive hash it provides
>> to the stack is reflected back in skb->hash for packets transmitted
>> on the associated connection.
>
> [...]
>>
>> +       if (unlikely(skb->reflect_hash && sk->sk_txhash != skb->hash))
>> +               sk->sk_txhash = skb->hash;
>
>
> Is there also an initial user setting this somewhere?

inet_set_txhash and ip6_set_txhash are called for connected sockets.
There is no user API to set these.

Tom

Re: [PATCH net-next] net: Reflect RX hash for transmit

[Daniel Borkmann]

On 02/15/2015 09:19 PM, Tom Herbert wrote:
> On Sun, Feb 15, 2015 at 11:59 AM, Daniel Borkmann <daniel@iogearbox.net> wrote:
>> On 02/15/2015 07:37 PM, Tom Herbert wrote:
>>>
>>> This patch allows a driver to request that the receive hash it provides
>>> to the stack is reflected back in skb->hash for packets transmitted
>>> on the associated connection.
>>
>> [...]
>>>
>>> +       if (unlikely(skb->reflect_hash && sk->sk_txhash != skb->hash))
>>> +               sk->sk_txhash = skb->hash;
>>
>>
>> Is there also an initial user setting this somewhere?
>
> inet_set_txhash and ip6_set_txhash are called for connected sockets.
> There is no user API to set these.

Yes sure, this was maybe a bit unclear; I was referring to reflect_hash.

Thanks,
Daniel

Re: [PATCH net-next] net: Reflect RX hash for transmit

[Eric Dumazet]

On Sun, 2015-02-15 at 10:37 -0800, Tom Herbert wrote:
> This patch allows a driver to request that the receive hash it provides
> to the stack is reflected back in skb->hash for packets transmitted
> on the associated connection. The hash value returned by a device could
> have meaning when used in transmit to identify a flow, for instance
> the hash may by a flow ID for a flow created in a (virtual) device.
> With the flow ID provided on transmit this can obviate the need
> to create the hash and do the lookup on the fly in transmit.
> 
> Note that this is an opportunistic optimization, there is no
> separate negotiation between the stack and driver/device for
> the hash used in either TX or RX.
> 
> Signed-off-by: Tom Herbert <therbert@google.com>
> ---

It is not clear to me why we would want to consume yet another skb->bit
for this.

This seems it might be a socket attribute, rather than a device provided
one on every packet.

Anyway, this seems a premature change, as no driver currently needs this
yet ?

Re: [PATCH net-next] net: Reflect RX hash for transmit

[David Miller]

From: Tom Herbert <therbert@google.com>
Date: Sun, 15 Feb 2015 10:37:18 -0800

> This patch allows a driver to request that the receive hash it provides
> to the stack is reflected back in skb->hash for packets transmitted
> on the associated connection. The hash value returned by a device could
> have meaning when used in transmit to identify a flow, for instance
> the hash may by a flow ID for a flow created in a (virtual) device.
> With the flow ID provided on transmit this can obviate the need
> to create the hash and do the lookup on the fly in transmit.
> 
> Note that this is an opportunistic optimization, there is no
> separate negotiation between the stack and driver/device for
> the hash used in either TX or RX.
> 
> Signed-off-by: Tom Herbert <therbert@google.com>

Like Eric, I'd like to see this resubmitted alongside an actual user
of the new facility.

Thanks!