diff mbox series

[1/2] net: forbid the reentrant RX

Message ID 20200722085747.6514-1-jasowang@redhat.com (mailing list archive)
State New, archived
Headers show
Series [1/2] net: forbid the reentrant RX | expand

Commit Message

Jason Wang July 22, 2020, 8:57 a.m. UTC
The memory API allows DMA into NIC's MMIO area. This means the NIC's
RX routine must be reentrant. Instead of auditing all the NIC, we can
simply detect the reentrancy and return early. The queue->delivering
is set and cleared by qemu_net_queue_deliver() for other queue helpers
to know whether the delivering in on going (NIC's receive is being
called). We can check it and return early in qemu_net_queue_flush() to
forbid reentrant RX.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 net/queue.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Jason Wang July 28, 2020, 4 a.m. UTC | #1
On 2020/7/22 下午4:57, Jason Wang wrote:
> The memory API allows DMA into NIC's MMIO area. This means the NIC's
> RX routine must be reentrant. Instead of auditing all the NIC, we can
> simply detect the reentrancy and return early. The queue->delivering
> is set and cleared by qemu_net_queue_deliver() for other queue helpers
> to know whether the delivering in on going (NIC's receive is being
> called). We can check it and return early in qemu_net_queue_flush() to
> forbid reentrant RX.
>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
>   net/queue.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/net/queue.c b/net/queue.c
> index 0164727e39..19e32c80fd 100644
> --- a/net/queue.c
> +++ b/net/queue.c
> @@ -250,6 +250,9 @@ void qemu_net_queue_purge(NetQueue *queue, NetClientState *from)
>   
>   bool qemu_net_queue_flush(NetQueue *queue)
>   {
> +    if (queue->delivering)
> +        return false;
> +
>       while (!QTAILQ_EMPTY(&queue->packets)) {
>           NetPacket *packet;
>           int ret;


Queued for rc2.

Thanks
Alexander Bulekov Sept. 2, 2020, 3:56 p.m. UTC | #2
On 200728 1200, Jason Wang wrote:
> 
> On 2020/7/22 下午4:57, Jason Wang wrote:
> > The memory API allows DMA into NIC's MMIO area. This means the NIC's
> > RX routine must be reentrant. Instead of auditing all the NIC, we can
> > simply detect the reentrancy and return early. The queue->delivering
> > is set and cleared by qemu_net_queue_deliver() for other queue helpers
> > to know whether the delivering in on going (NIC's receive is being
> > called). We can check it and return early in qemu_net_queue_flush() to
> > forbid reentrant RX.
> > 
> > Signed-off-by: Jason Wang <jasowang@redhat.com>
> > ---
> >   net/queue.c | 3 +++
> >   1 file changed, 3 insertions(+)
> > 
> > diff --git a/net/queue.c b/net/queue.c
> > index 0164727e39..19e32c80fd 100644
> > --- a/net/queue.c
> > +++ b/net/queue.c
> > @@ -250,6 +250,9 @@ void qemu_net_queue_purge(NetQueue *queue, NetClientState *from)
> >   bool qemu_net_queue_flush(NetQueue *queue)
> >   {
> > +    if (queue->delivering)
> > +        return false;
> > +
> >       while (!QTAILQ_EMPTY(&queue->packets)) {
> >           NetPacket *packet;
> >           int ret;
> 
> 
> Queued for rc2.
> 
> Thanks
> 

Hi Jason,
I don't think this ever made it in. Are there any remaining problems?
Thanks
-Alex
Jason Wang Sept. 3, 2020, 3:56 a.m. UTC | #3
On 2020/9/2 下午11:56, Alexander Bulekov wrote:
> On 200728 1200, Jason Wang wrote:
>> On 2020/7/22 下午4:57, Jason Wang wrote:
>>> The memory API allows DMA into NIC's MMIO area. This means the NIC's
>>> RX routine must be reentrant. Instead of auditing all the NIC, we can
>>> simply detect the reentrancy and return early. The queue->delivering
>>> is set and cleared by qemu_net_queue_deliver() for other queue helpers
>>> to know whether the delivering in on going (NIC's receive is being
>>> called). We can check it and return early in qemu_net_queue_flush() to
>>> forbid reentrant RX.
>>>
>>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>>> ---
>>>    net/queue.c | 3 +++
>>>    1 file changed, 3 insertions(+)
>>>
>>> diff --git a/net/queue.c b/net/queue.c
>>> index 0164727e39..19e32c80fd 100644
>>> --- a/net/queue.c
>>> +++ b/net/queue.c
>>> @@ -250,6 +250,9 @@ void qemu_net_queue_purge(NetQueue *queue, NetClientState *from)
>>>    bool qemu_net_queue_flush(NetQueue *queue)
>>>    {
>>> +    if (queue->delivering)
>>> +        return false;
>>> +
>>>        while (!QTAILQ_EMPTY(&queue->packets)) {
>>>            NetPacket *packet;
>>>            int ret;
>>
>> Queued for rc2.
>>
>> Thanks
>>
> Hi Jason,
> I don't think this ever made it in. Are there any remaining problems?
> Thanks
> -Alex


Hi Alex:

It should have been merged:

https://git.qemu.org/?p=qemu.git;a=commit;h=22dc8663d9fc7baa22100544c600b6285a63c7a3

Thanks


>
Alexander Bulekov Sept. 3, 2020, 4:02 a.m. UTC | #4
On 200903 1156, Jason Wang wrote:
> 
> On 2020/9/2 下午11:56, Alexander Bulekov wrote:
> > On 200728 1200, Jason Wang wrote:
> > > On 2020/7/22 下午4:57, Jason Wang wrote:
> > > > The memory API allows DMA into NIC's MMIO area. This means the NIC's
> > > > RX routine must be reentrant. Instead of auditing all the NIC, we can
> > > > simply detect the reentrancy and return early. The queue->delivering
> > > > is set and cleared by qemu_net_queue_deliver() for other queue helpers
> > > > to know whether the delivering in on going (NIC's receive is being
> > > > called). We can check it and return early in qemu_net_queue_flush() to
> > > > forbid reentrant RX.
> > > > 
> > > > Signed-off-by: Jason Wang <jasowang@redhat.com>
> > > > ---
> > > >    net/queue.c | 3 +++
> > > >    1 file changed, 3 insertions(+)
> > > > 
> > > > diff --git a/net/queue.c b/net/queue.c
> > > > index 0164727e39..19e32c80fd 100644
> > > > --- a/net/queue.c
> > > > +++ b/net/queue.c
> > > > @@ -250,6 +250,9 @@ void qemu_net_queue_purge(NetQueue *queue, NetClientState *from)
> > > >    bool qemu_net_queue_flush(NetQueue *queue)
> > > >    {
> > > > +    if (queue->delivering)
> > > > +        return false;
> > > > +
> > > >        while (!QTAILQ_EMPTY(&queue->packets)) {
> > > >            NetPacket *packet;
> > > >            int ret;
> > > 
> > > Queued for rc2.
> > > 
> > > Thanks
> > > 
> > Hi Jason,
> > I don't think this ever made it in. Are there any remaining problems?
> > Thanks
> > -Alex
> 
> 
> Hi Alex:
> 
> It should have been merged:
> 
> https://git.qemu.org/?p=qemu.git;a=commit;h=22dc8663d9fc7baa22100544c600b6285a63c7a3
> 
> Thanks
> 

Ah. I missed only 1/2 was queued. I guess the e1000 patch didn't make
the cut..
Thanks
-Alex

> 
> > 
>
diff mbox series

Patch

diff --git a/net/queue.c b/net/queue.c
index 0164727e39..19e32c80fd 100644
--- a/net/queue.c
+++ b/net/queue.c
@@ -250,6 +250,9 @@  void qemu_net_queue_purge(NetQueue *queue, NetClientState *from)
 
 bool qemu_net_queue_flush(NetQueue *queue)
 {
+    if (queue->delivering)
+        return false;
+
     while (!QTAILQ_EMPTY(&queue->packets)) {
         NetPacket *packet;
         int ret;