[Qemu-devel] [PATCH] xen: Drop net_rx_ok
This is necessary because once we return false from .can_receive, we
need to flush the queue when the .can_receive conditions become true
again, (for example when more buffer is available).
We can rely on net_rx_packet (which checks the same conditions) to drop
the packet if the device is not ready, so drop net_xen_info.can_receive.
Signed-off-by: Fam Zheng
---
hw/net/xen_nic.c | 22 --
1 file changed, 22 deletions(-)
diff --git a/hw/net/xen_nic.c b/hw/net/xen_nic.c
index 19ecfc4..da9edc0 100644
--- a/hw/net/xen_nic.c
+++ b/hw/net/xen_nic.c
@@ -234,27 +234,6 @@ static void net_rx_response(struct XenNetDev *netdev,
#define NET_IP_ALIGN 2
-static int net_rx_ok(NetClientState *nc)
-{
-struct XenNetDev *netdev = qemu_get_nic_opaque(nc);
-RING_IDX rc, rp;
-
-if (netdev->xendev.be_state != XenbusStateConnected) {
-return 0;
-}
-
-rc = netdev->rx_ring.req_cons;
-rp = netdev->rx_ring.sring->req_prod;
-xen_rmb();
-
-if (rc == rp || RING_REQUEST_CONS_OVERFLOW(&netdev->rx_ring, rc)) {
-xen_be_printf(&netdev->xendev, 2, "%s: no rx buffers (%d/%d)\n",
- __FUNCTION__, rc, rp);
-return 0;
-}
-return 1;
-}
-
static ssize_t net_rx_packet(NetClientState *nc, const uint8_t *buf, size_t
size)
{
struct XenNetDev *netdev = qemu_get_nic_opaque(nc);
@@ -304,7 +283,6 @@ static ssize_t net_rx_packet(NetClientState *nc, const
uint8_t *buf, size_t size
static NetClientInfo net_xen_info = {
.type = NET_CLIENT_OPTIONS_KIND_NIC,
.size = sizeof(NICState),
-.can_receive = net_rx_ok,
.receive = net_rx_packet,
};
--
2.4.4
Re: [Qemu-devel] [PATCH] xen: Drop net_rx_ok
On Mon, Jul 20, 2015 at 06:12:09PM +0100, Stefan Hajnoczi wrote:
> On Thu, Jul 02, 2015 at 01:39:16PM +0100, Stefan Hajnoczi wrote:
> > On Tue, Jun 30, 2015 at 10:42:37AM +0800, Fam Zheng wrote:
> > > This is necessary because once we return false from .can_receive, we
> > > need to flush the queue when the .can_receive conditions become true
> > > again, (for example when more buffer is available).
> > >
> > > We can rely on net_rx_packet (which checks the same conditions) to drop
> > > the packet if the device is not ready, so drop net_xen_info.can_receive.
> >
> > This patch changes behavior:
> >
> > Previously can_receive() false meant packets are queued.
> >
> > Now those same conditions result in net_rx_packet() returning -1, so
> > packets are discarded.
> >
> > In order to keep the spirit of the queuing mechanism - where we tell a
> > sender to hold off until more rx buffers become available - I think the
> > following line in net_rx_packet() needs to be changed:
> >
> > if (rc == rp || RING_REQUEST_CONS_OVERFLOW(&netdev->rx_ring, rc)) {
> > xen_be_printf(&netdev->xendev, 2, "no buffer, drop packet\n");
> > return -1; <-- this should be changed to return 0
> > }
> >
> > That change assumes that net_event()'s flush is always called when the
> > rx ring gets more free space.
> >
> > Any thoughts from Xen folks?
>
> Ping?
Need input from Xen developers.
Ping?
pgp1oD0mETHph.pgp
Description: PGP signature
Re: [Qemu-devel] [PATCH] xen: Drop net_rx_ok
On 7/27/15 21:16, Stefan Hajnoczi wrote:
> On Mon, Jul 20, 2015 at 06:12:09PM +0100, Stefan Hajnoczi wrote:
>> On Thu, Jul 02, 2015 at 01:39:16PM +0100, Stefan Hajnoczi wrote:
>>> On Tue, Jun 30, 2015 at 10:42:37AM +0800, Fam Zheng wrote:
This is necessary because once we return false from .can_receive, we
need to flush the queue when the .can_receive conditions become true
again, (for example when more buffer is available).
We can rely on net_rx_packet (which checks the same conditions) to drop
the packet if the device is not ready, so drop net_xen_info.can_receive.
>>>
>>> This patch changes behavior:
>>>
>>> Previously can_receive() false meant packets are queued.
>>>
>>> Now those same conditions result in net_rx_packet() returning -1, so
>>> packets are discarded.
>>>
>>> In order to keep the spirit of the queuing mechanism - where we tell a
>>> sender to hold off until more rx buffers become available - I think the
>>> following line in net_rx_packet() needs to be changed:
>>>
>>> if (rc == rp || RING_REQUEST_CONS_OVERFLOW(&netdev->rx_ring, rc)) {
>>> xen_be_printf(&netdev->xendev, 2, "no buffer, drop packet\n");
>>> return -1; <-- this should be changed to return 0
>>> }
>>>
>>> That change assumes that net_event()'s flush is always called when the
>>> rx ring gets more free space.
>>>
>>> Any thoughts from Xen folks?
>>
>> Ping?
>
> Need input from Xen developers.
>
> Ping?
>
In fact, I am not Xen folks. But I shall provide my idea for it, since
Cc to me.
For me, it is reasonable to use "return 0" instead of "return -1".
- It completes the current API and keep compatible with the original
usage. As return size, it needs to consider about all numbers (zero,
negative, and positive numbers, like e.g. standard read/write API).
- Comparing with all the other NetClientInfo.receive providers, some of
them consider about it (e.g. virtio), others not (maybe each of them
have their own reasons -- e.g. hardware will never generate 0 size).
Welcome any other members' ideas, suggestions and completions.
Thanks.
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH] xen: Drop net_rx_ok
On Mon, 27 Jul 2015, Stefan Hajnoczi wrote:
> On Mon, Jul 20, 2015 at 06:12:09PM +0100, Stefan Hajnoczi wrote:
> > On Thu, Jul 02, 2015 at 01:39:16PM +0100, Stefan Hajnoczi wrote:
> > > On Tue, Jun 30, 2015 at 10:42:37AM +0800, Fam Zheng wrote:
> > > > This is necessary because once we return false from .can_receive, we
> > > > need to flush the queue when the .can_receive conditions become true
> > > > again, (for example when more buffer is available).
> > > >
> > > > We can rely on net_rx_packet (which checks the same conditions) to drop
> > > > the packet if the device is not ready, so drop net_xen_info.can_receive.
> > >
> > > This patch changes behavior:
> > >
> > > Previously can_receive() false meant packets are queued.
> > >
> > > Now those same conditions result in net_rx_packet() returning -1, so
> > > packets are discarded.
> > >
> > > In order to keep the spirit of the queuing mechanism - where we tell a
> > > sender to hold off until more rx buffers become available - I think the
> > > following line in net_rx_packet() needs to be changed:
> > >
> > > if (rc == rp || RING_REQUEST_CONS_OVERFLOW(&netdev->rx_ring, rc)) {
> > > xen_be_printf(&netdev->xendev, 2, "no buffer, drop packet\n");
> > > return -1; <-- this should be changed to return 0
> > > }
> > >
> > > That change assumes that net_event()'s flush is always called when the
> > > rx ring gets more free space.
> > >
> > > Any thoughts from Xen folks?
> >
> > Ping?
>
> Need input from Xen developers.
FYI I don't have an opinion on this: I am not familiar with this code as
xen_nic is the one Xen PV backend in QEMU that is not used with Xen.
Theoretically it could be used, but actually there is no way to select
it from the toolstack.
Re: [Qemu-devel] [PATCH] xen: Drop net_rx_ok
On Thu, Jul 02, 2015 at 01:39:16PM +0100, Stefan Hajnoczi wrote:
> On Tue, Jun 30, 2015 at 10:42:37AM +0800, Fam Zheng wrote:
> > This is necessary because once we return false from .can_receive, we
> > need to flush the queue when the .can_receive conditions become true
> > again, (for example when more buffer is available).
> >
> > We can rely on net_rx_packet (which checks the same conditions) to drop
> > the packet if the device is not ready, so drop net_xen_info.can_receive.
>
> This patch changes behavior:
>
> Previously can_receive() false meant packets are queued.
>
> Now those same conditions result in net_rx_packet() returning -1, so
> packets are discarded.
>
> In order to keep the spirit of the queuing mechanism - where we tell a
> sender to hold off until more rx buffers become available - I think the
> following line in net_rx_packet() needs to be changed:
>
> if (rc == rp || RING_REQUEST_CONS_OVERFLOW(&netdev->rx_ring, rc)) {
> xen_be_printf(&netdev->xendev, 2, "no buffer, drop packet\n");
> return -1; <-- this should be changed to return 0
> }
>
> That change assumes that net_event()'s flush is always called when the
> rx ring gets more free space.
>
> Any thoughts from Xen folks?
Ping?
pgpSw76b8TbI6.pgp
Description: PGP signature
Re: [Qemu-devel] [PATCH] xen: Drop net_rx_ok
On Tue, Jun 30, 2015 at 10:42:37AM +0800, Fam Zheng wrote:
> This is necessary because once we return false from .can_receive, we
> need to flush the queue when the .can_receive conditions become true
> again, (for example when more buffer is available).
>
> We can rely on net_rx_packet (which checks the same conditions) to drop
> the packet if the device is not ready, so drop net_xen_info.can_receive.
This patch changes behavior:
Previously can_receive() false meant packets are queued.
Now those same conditions result in net_rx_packet() returning -1, so
packets are discarded.
In order to keep the spirit of the queuing mechanism - where we tell a
sender to hold off until more rx buffers become available - I think the
following line in net_rx_packet() needs to be changed:
if (rc == rp || RING_REQUEST_CONS_OVERFLOW(&netdev->rx_ring, rc)) {
xen_be_printf(&netdev->xendev, 2, "no buffer, drop packet\n");
return -1; <-- this should be changed to return 0
}
That change assumes that net_event()'s flush is always called when the
rx ring gets more free space.
Any thoughts from Xen folks?
pgpnIfmr2yqBn.pgp
Description: PGP signature
