Re: [Qemu-devel] [PATCH v5] net: L2TPv3 transport

["Anton Ivanov (antivano)" <antivano@cisco.com>]

[snip]
>> So the fact that qemu_send_packet_async() has returned a non-zero does not mean that we have not paid the price for it :) 
> A non-zero return is simply an error code from the ->receive() function.
> In this case the packet is dropped but queuing is unaffected.
>
>> The relevant code is in qemu_net_queue_append which invoked by
>> qemu_net_queue_send which is what qemu_send_packet_async calls.
> Are we interpreting the code differently?

Let me go through this once again and make sure it has the correct logic
in RX and TX direction.

In RX it should not use any of the queue.c functions as the message
vector doubles up for queue.

In TX it should still use them and still have _flush invoked where
needed as it is not using sendmmsg for now.

A.


>
>>>> +static void l2tpv3_writable(void *opaque)
>>>> +{
>>>> +    NetL2TPV3State *s = opaque;
>>>> +
>>>> +    l2tpv3_write_poll(s, false);
>>>> +
>>>> +    net_l2tpv3_send(s);
>>> This is the wrong function because net-l2tpv3_send() *reads* new packets
>>> from the socket.  We must tell the peer to resume transmitting packets
>>> so we can *write* them to the socket.
>>>
>>> This line should be:
>>> qemu_flush_queued_packets(&s->nc);
>> No.
>>
>> That function doubles for local buffer using the recvmmsg vector, there
>> is never any packets in the queue from queue.c to flush. The exact idea
>> here is to eliminate that queue as it is surplus to requirements if you
>> are doing multirx. If you are doing multirx (or packet mmap) your
>> packets are in a multi-packet buffer already. It is a perfect queue,
>> there is no need for another :)
>>
>> The send function is written so it picks up where it has left off last
>> time so any packets left over in the recvmmsg vector are flushed first
>> before any new reads.
> You're thinking about the wrong data transfer direction.  There are two
> queues:
> 1. recvmmsg() -> NIC incoming queue - you are trying to bypass this.
> 2. sendmsg() -> l2tpv3's incoming queue - if sendmsg() fails with EAGAIN.
>
> l2tpv3_writable() is invoked when the socket becomes writable (case #2)
> and we can resume sendmsg().  This has nothing to do with recvmmsg()
> (case #1).

My exact thought, going through it at the moment.

>
> In other words, the host kernel stopped us from transmitting packets
> because it ran out of buffer space.  So the NIC placed packets into
> l2tpv3's incoming queue and now we need to flush them, as well as
> re-enabling transmission.
>

[snip]

A.