svn commit: r341586 - head/sys/dev/mlx5/mlx5_en

[Slava Shwartsman]

On 17-Apr-19 00:28, John Baldwin wrote:
> On 4/16/19 8:32 AM, Hans Petter Selasky wrote:
>> On 4/16/19 4:39 PM, Andrey V. Elsukov wrote:
>>> On 05.12.2018 17:25, Slava Shwartsman wrote:
>>>> Author: slavash
>>>> Date: Wed Dec  5 14:25:03 2018
>>>> New Revision: 341586
>>>> URL: https://svnweb.freebsd.org/changeset/base/341586
>>>>
>>>> Log:
>>>>     mlx5en: Implement backpressure indication.
>>>>     
>>>>     The backpressure indication is implemented using an unlimited rate type of
>>>>     mbuf send tag. When the upper layers typically the socket layer has obtained such
>>>>     a tag, it can then query the destination driver queue for the current
>>>>     amount of space available in the send queue.
>>>>     
>>>>     A single mbuf send tag may be referenced multiple times and a refcount has been added
>>>>     to the mlx5e_priv structure to track its usage. Because the send tag resides
>>>>     in the mlx5e_channel structure, there is no need to wait for refcounts to reach
>>>>     zero until the mlx4en(4) driver is detached. The channels structure is persistant
>>>>     during the lifetime of the mlx5en(4) driver it belongs to and can so be accessed
>>>>     without any need of synchronization.
>>>>     
>>>>     The mlx5e_snd_tag structure was extended to contain a type field, because there are now
>>>>     two different tag types which end up in the driver which need to be distinguished.
>>>>     
>>>>     Submitted by:   hselasky@
>>>>     Approved by:    hselasky (mentor)
>>>>     MFC after:      1 week
>>>>     Sponsored by:   Mellanox Technologies
>>>> @@ -587,27 +609,33 @@ mlx5e_xmit(struct ifnet *ifp, struct mbuf *mb)
>>>>    	struct mlx5e_sq *sq;
>>>>    	int ret;
>>>>    
>>>> -	sq = mlx5e_select_queue(ifp, mb);
>>>> -	if (unlikely(sq == NULL)) {
>>>> -#ifdef RATELIMIT
>>>> -		/* Check for route change */
>>>> -		if (mb->m_pkthdr.snd_tag != NULL &&
>>>> -		    mb->m_pkthdr.snd_tag->ifp != ifp) {
>>>> +	if (mb->m_pkthdr.snd_tag != NULL) {
>>>> +		sq = mlx5e_select_queue_by_send_tag(ifp, mb);
>>>> +		if (unlikely(sq == NULL)) {
>>>> +			/* Check for route change */
>>>> +			if (mb->m_pkthdr.snd_tag->ifp != ifp) {
>>>> +				/* Free mbuf */
>>>> +				m_freem(mb);
>>>> +
>>>> +				/*
>>>> +				 * Tell upper layers about route
>>>> +				 * change and to re-transmit this
>>>> +				 * packet:
>>>> +				 */
>>>> +				return (EAGAIN);
>>>> +			}
>>>
>>> Hi,
>>>
>>> I just discovered something strange and found that this commit is the
>>> cause.
>>> The test system has mlx5en 100G interface. It has two vlans: vlan500 and
>>> vlan100.
>>> Via vlan500 it receives some packets flows. Then it routes these packets
>>> into vlan100.
>>> But packets are dropped in mlx5e_xmit() with EAGAIN error code.
>>>
>>> # dtrace -n 'fbt::ip6_output:return {printf("%d", arg1);}'
>>> dtrace: description 'fbt::ip6_output:return ' matched 1 probe
>>> CPU     ID                    FUNCTION:NAME
>>>    23  54338                ip6_output:return 35
>>>    16  54338                ip6_output:return 35
>>>    21  54338                ip6_output:return 35
>>>    22  54338                ip6_output:return 35
>>>    24  54338                ip6_output:return 35
>>>    23  54338                ip6_output:return 35
>>>    14  54338                ip6_output:return 35
>>> ^C
>>>
>>> # dtrace -n 'fbt::mlx5e_xmit:return {printf("%d", arg1);}'
>>> dtrace: description 'fbt::mlx5e_xmit:return ' matched 1 probe
>>> CPU     ID                    FUNCTION:NAME
>>>    16  69030                mlx5e_xmit:return 35
>>>    23  69030                mlx5e_xmit:return 35
>>>    26  69030                mlx5e_xmit:return 35
>>>    25  69030                mlx5e_xmit:return 35
>>>    24  69030                mlx5e_xmit:return 35
>>>    21  69030                mlx5e_xmit:return 35
>>>    26  69030                mlx5e_xmit:return 35
>>> ^C
>>>
>>> The kernel config is GENERIC.
>>> 13.0-CURRENT #9 r345758+82f3d57(svn_head)-dirty
>>>
>>
>> Hi,
>>
>> This might be a case where rcvif in the mbuf's pktheader is not cleared
>> before the packet is fed back on the wire.
>>
>> John Baldwin is working on the send tags implementation, to eliminate
>> the EAGAIN handling in the network drivers.
> 
> I will try to push this branch sooner then since it affects more than just
> TLS.  Part of the change includes a new flag we can use to assert that we
Thanks John!
> aren't just getting a stale rcvif (though there are also now assertions in
> ip_output that should catch this case I think).
> 



Hi Andrey,

Yes, we were able to reproduce this issue in house. If you don't mind, I 
prefer to wait for John's update - where he eliminates the EAGAIN 
handling in the network drivers.


Slava