Re: [net-next PATCH v3 3/5] net: Enable Tx queue selection based on Rx queues

["Nambiar, Amritha" <amritha.nambiar@intel.com>]

On 6/6/2018 12:13 PM, Willem de Bruijn wrote:
> On Wed, Jun 6, 2018 at 3:08 PM, Samudrala, Sridhar
> <sridhar.samudrala@intel.com> wrote:
>>
>> On 6/6/2018 11:56 AM, Willem de Bruijn wrote:
>>>
>>> On Tue, Jun 5, 2018 at 4:38 AM, Amritha Nambiar
>>> <amritha.nambiar@intel.com> wrote:
>>>>
>>>> This patch adds support to pick Tx queue based on the Rx queue(s) map
>>>> configuration set by the admin through the sysfs attribute
>>>> for each Tx queue. If the user configuration for receive queue(s) map
>>>> does not apply, then the Tx queue selection falls back to CPU(s) map
>>>> based selection and finally to hashing.
>>>>
>>>> Signed-off-by: Amritha Nambiar <amritha.nambiar@intel.com>
>>>> Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
>>>> ---
>>>>   int sysctl_tcp_max_orphans __read_mostly = NR_FILE;
>>>>
>>>> @@ -5574,6 +5575,7 @@ void tcp_finish_connect(struct sock *sk, struct
>>>> sk_buff *skb)
>>>>          if (skb) {
>>>>                  icsk->icsk_af_ops->sk_rx_dst_set(sk, skb);
>>>>                  security_inet_conn_established(sk, skb);
>>>> +               sk_mark_napi_id(sk, skb);
>>>>          }
>>>
>>> This and the call below should be in a standalone patch, as the mark
>>> changes are not rxq-xps specific. Is the additional earlier marking really
>>> required?
>>
>>
>> The additional earlier marking in tcp_finish_connect() allows a client app
>> to do
>> SO_INCOMING_NAPI_ID after a a connect() call to get the right queue
>> association
>> for a socket.
>>
>> The marking in tcp_conn_request() allows syn-ack to go on the right tx-queue
>> associated with the queue on which syn is received.
> 
> I understand the intent. My question really is whether it is needed.
> Marking has been slightly lossy in this regard in the past, not
> necessarily as an oversight. I don't mean to make that call here,
> but it's worth discussion and its own patch.
> 
Will separate this out into a standalone patch in v4.