Xu, Dongxiao
2009-Sep-30 07:21 UTC
[Xen-devel][PV-ops][PATCH 1/2] VNIF(netback): Using smart polling instead of event notification.
- Netback will not notify netfront until it finds that the netfront has
stopped polling.
- Netback will set a flag in xenstore to indicate whether netback
supports the smart polling feature. If there is only one side
supporting it, the communication mechanism will fall back to default,
and the new feature will not be used. The feature is enabled only
when both sides have the flag set in xenstore.
Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
---
drivers/xen/netback/common.h | 2 ++
drivers/xen/netback/netback.c | 23 +++++++++++++++++++++--
drivers/xen/netback/xenbus.c | 16 ++++++++++++++++
include/xen/interface/io/ring.h | 3 ++-
4 files changed, 41 insertions(+), 3 deletions(-)
diff --git a/drivers/xen/netback/common.h b/drivers/xen/netback/common.h
index 9056be0..b15a7e1 100644
--- a/drivers/xen/netback/common.h
+++ b/drivers/xen/netback/common.h
@@ -76,6 +76,8 @@ struct xen_netif {
/* Set of features that can be turned on in dev->features. */
int features;
+ int smart_poll;
+
/* Internal feature information. */
u8 can_queue:1; /* can queue packets for receiver? */
diff --git a/drivers/xen/netback/netback.c b/drivers/xen/netback/netback.c
index d7d738e..869b6cc 100644
--- a/drivers/xen/netback/netback.c
+++ b/drivers/xen/netback/netback.c
@@ -640,7 +640,8 @@ static void net_rx_action(unsigned long unused)
RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(&netif->rx, ret);
irq = netif->irq;
- if (ret && !rx_notify[irq]) {
+ if (ret && !rx_notify[irq] &&
+ (netif->smart_poll != 1)) {
rx_notify[irq] = 1;
notify_list[notify_nr++] = irq;
}
@@ -650,6 +651,16 @@ static void net_rx_action(unsigned long unused)
!netbk_queue_full(netif))
netif_wake_queue(netif->dev);
+ /* netfront_smartpoll_active indicates whether netfront timer
+ * is active.
+ */
+ if ((netif->smart_poll == 1)) {
+ if (!(netif->rx.sring->netfront_smartpoll_active)) {
+ notify_remote_via_irq(irq);
+ netif->rx.sring->netfront_smartpoll_active = 1;
+ }
+ }
+
netif_put(netif);
dev_kfree_skb(skb);
npo.meta_cons += nr_frags + 1;
@@ -1471,7 +1482,15 @@ static void make_tx_response(struct xen_netif *netif,
netif->tx.rsp_prod_pvt = ++i;
RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(&netif->tx, notify);
- if (notify)
+ /* netfront_smartpoll_active indicates whether netfront timer is
+ * active.
+ */
+ if ((netif->smart_poll == 1)) {
+ if (!(netif->rx.sring->netfront_smartpoll_active)) {
+ notify_remote_via_irq(netif->irq);
+ netif->rx.sring->netfront_smartpoll_active = 1;
+ }
+ } else if (notify)
notify_remote_via_irq(netif->irq);
}
diff --git a/drivers/xen/netback/xenbus.c b/drivers/xen/netback/xenbus.c
index a492288..3d8ed9a 100644
--- a/drivers/xen/netback/xenbus.c
+++ b/drivers/xen/netback/xenbus.c
@@ -115,6 +115,14 @@ static int netback_probe(struct xenbus_device *dev,
goto abort_transaction;
}
+ /* We support data smart poll mechanism */
+ err = xenbus_printf(xbt, dev->nodename,
+ "feature-smart-poll", "%d", 1);
+ if (err) {
+ message = "writing feature-smart-poll";
+ goto abort_transaction;
+ }
+
err = xenbus_transaction_end(xbt, 0);
} while (err == -EAGAIN);
@@ -415,6 +423,14 @@ static int connect_rings(struct backend_info *be)
be->netif->dev->features &= ~NETIF_F_IP_CSUM;
}
+ if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-smart-poll",
+ "%d", &val) < 0)
+ val = 0;
+ if (val)
+ be->netif->smart_poll = 1;
+ else
+ be->netif->smart_poll = 0;
+
/* Map the shared frame, irq etc. */
err = netif_map(be->netif, tx_ring_ref, rx_ring_ref, evtchn);
if (err) {
diff --git a/include/xen/interface/io/ring.h b/include/xen/interface/io/ring.h
index e8cbf43..865dcf0 100644
--- a/include/xen/interface/io/ring.h
+++ b/include/xen/interface/io/ring.h
@@ -73,7 +73,8 @@ union __name##_sring_entry { \
struct __name##_sring { \
RING_IDX req_prod, req_event; \
RING_IDX rsp_prod, rsp_event; \
- uint8_t pad[48]; \
+ uint8_t netfront_smartpoll_active; \
+ uint8_t pad[47]; \
union __name##_sring_entry ring[1]; /* variable-length */ \
}; \
\
--
1.6.3
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
James Harper
2009-Sep-30 09:27 UTC
RE: [Xen-devel][PV-ops][PATCH 1/2] VNIF(netback): Using smart pollinginstead of event notification.
> > + /* netfront_smartpoll_active indicates whether netfronttimer> + * is active. > + */ > + if ((netif->smart_poll == 1)) { > + if(!(netif->rx.sring->netfront_smartpoll_active)) {> + notify_remote_via_irq(irq); > +netif->rx.sring->netfront_smartpoll_active = 1;> + } > + }I think that the frontend should use the event field to turn off notification here. It should set it to the number of outstanding rx slots less some number, eg if it had filled the ring with rx slots (eg 256), then prod + 128 might be a good number. That way if a lot of small packets come it, the frontend will still get notified if the ring is half empty even if the 1Khz timer wasn''t elapsed yet. Otherwise the rx ring could starve.> netif->tx.rsp_prod_pvt = ++i; > RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(&netif->tx, notify); > - if (notify) > + /* netfront_smartpoll_active indicates whether netfront timer is > + * active. > + */ > + if ((netif->smart_poll == 1)) { > + if (!(netif->rx.sring->netfront_smartpoll_active)) { > + notify_remote_via_irq(netif->irq); > + netif->rx.sring->netfront_smartpoll_active = 1; > + } > + } else if (notify) > notify_remote_via_irq(netif->irq); > }I''m not so sure about this either. The frontend knows how many outstanding tx packets it has put on the ring, and it is reasonable to assume that they will be sent in a timely manner, so I think in this case the frontend is in the best position to set the event to an acceptable value and the backend will notify on that basis. James _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Xu, Dongxiao
2009-Sep-30 17:29 UTC
RE: [Xen-devel][PV-ops][PATCH 1/2] VNIF(netback): Using smart pollinginstead of event notification.
See below comments. Thanks! Dongxiao ________________________________________ From: James Harper [james.harper@bendigoit.com.au] Sent: Wednesday, September 30, 2009 2:27 AM To: Xu, Dongxiao; xen-devel@lists.xensource.com Subject: RE: [Xen-devel][PV-ops][PATCH 1/2] VNIF(netback): Using smart pollinginstead of event notification.> > + /* netfront_smartpoll_active indicates whether netfronttimer> + * is active. > + */ > + if ((netif->smart_poll == 1)) { > + if(!(netif->rx.sring->netfront_smartpoll_active)) {> + notify_remote_via_irq(irq); > +netif->rx.sring->netfront_smartpoll_active = 1;> + } > + }I think that the frontend should use the event field to turn off notification here. It should set it to the number of outstanding rx slots less some number, eg if it had filled the ring with rx slots (eg 256), then prod + 128 might be a good number. That way if a lot of small packets come it, the frontend will still get notified if the ring is half empty even if the 1Khz timer wasn''t elapsed yet. Otherwise the rx ring could starve. [Dongxiao]: In our patch, netfront will NOT be notified while polling data. Netback only notifies it once when data comes but netfront timer is not active. After the notification, netfront will do polling until there is no data coming during 100ms. So it eliminates most of the event channel notification.> netif->tx.rsp_prod_pvt = ++i; > RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(&netif->tx, notify); > - if (notify) > + /* netfront_smartpoll_active indicates whether netfront timer is > + * active. > + */ > + if ((netif->smart_poll == 1)) { > + if (!(netif->rx.sring->netfront_smartpoll_active)) { > + notify_remote_via_irq(netif->irq); > + netif->rx.sring->netfront_smartpoll_active = 1; > + } > + } else if (notify) > notify_remote_via_irq(netif->irq); > }I''m not so sure about this either. The frontend knows how many outstanding tx packets it has put on the ring, and it is reasonable to assume that they will be sent in a timely manner, so I think in this case the frontend is in the best position to set the event to an acceptable value and the backend will notify on that basis. [Dongxiao]: Actually our patch has eliminate most of the notification, so it is unnecessary for netfront to set the event field. As described before, netfront is only notified once when its timer is not active. Thanks! James _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Xu, Dongxiao
2009-Sep-30 18:17 UTC
[Xen-devel][PV-ops][PATCH 1/2] VNIF(netback): Using smart polling instead of event notification.
Resend the patch and CC Jeremy
- Netback will not notify netfront until it finds that the netfront has
stopped polling.
- Netback will set a flag in xenstore to indicate whether netback
supports the smart polling feature. If there is only one side
supporting it, the communication mechanism will fall back to default,
and the new feature will not be used. The feature is enabled only
when both sides have the flag set in xenstore.
Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
---
drivers/xen/netback/common.h | 2 ++
drivers/xen/netback/netback.c | 23 +++++++++++++++++++++--
drivers/xen/netback/xenbus.c | 16 ++++++++++++++++
include/xen/interface/io/ring.h | 3 ++-
4 files changed, 41 insertions(+), 3 deletions(-)
diff --git a/drivers/xen/netback/common.h b/drivers/xen/netback/common.h
index 9056be0..b15a7e1 100644
--- a/drivers/xen/netback/common.h
+++ b/drivers/xen/netback/common.h
@@ -76,6 +76,8 @@ struct xen_netif {
/* Set of features that can be turned on in dev->features. */
int features;
+ int smart_poll;
+
/* Internal feature information. */
u8 can_queue:1; /* can queue packets for receiver? */
diff --git a/drivers/xen/netback/netback.c b/drivers/xen/netback/netback.c
index d7d738e..869b6cc 100644
--- a/drivers/xen/netback/netback.c
+++ b/drivers/xen/netback/netback.c
@@ -640,7 +640,8 @@ static void net_rx_action(unsigned long unused)
RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(&netif->rx, ret);
irq = netif->irq;
- if (ret && !rx_notify[irq]) {
+ if (ret && !rx_notify[irq] &&
+ (netif->smart_poll != 1)) {
rx_notify[irq] = 1;
notify_list[notify_nr++] = irq;
}
@@ -650,6 +651,16 @@ static void net_rx_action(unsigned long unused)
!netbk_queue_full(netif))
netif_wake_queue(netif->dev);
+ /* netfront_smartpoll_active indicates whether netfront timer
+ * is active.
+ */
+ if ((netif->smart_poll == 1)) {
+ if (!(netif->rx.sring->netfront_smartpoll_active))
{
+ notify_remote_via_irq(irq);
+ netif->rx.sring->netfront_smartpoll_active
= 1;
+ }
+ }
+
netif_put(netif);
dev_kfree_skb(skb);
npo.meta_cons += nr_frags + 1;
@@ -1471,7 +1482,15 @@ static void make_tx_response(struct xen_netif *netif,
netif->tx.rsp_prod_pvt = ++i;
RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(&netif->tx, notify);
- if (notify)
+ /* netfront_smartpoll_active indicates whether netfront timer is
+ * active.
+ */
+ if ((netif->smart_poll == 1)) {
+ if (!(netif->rx.sring->netfront_smartpoll_active)) {
+ notify_remote_via_irq(netif->irq);
+ netif->rx.sring->netfront_smartpoll_active = 1;
+ }
+ } else if (notify)
notify_remote_via_irq(netif->irq);
}
diff --git a/drivers/xen/netback/xenbus.c b/drivers/xen/netback/xenbus.c
index a492288..3d8ed9a 100644
--- a/drivers/xen/netback/xenbus.c
+++ b/drivers/xen/netback/xenbus.c
@@ -115,6 +115,14 @@ static int netback_probe(struct xenbus_device *dev,
goto abort_transaction;
}
+ /* We support data smart poll mechanism */
+ err = xenbus_printf(xbt, dev->nodename,
+ "feature-smart-poll",
"%d", 1);
+ if (err) {
+ message = "writing feature-smart-poll";
+ goto abort_transaction;
+ }
+
err = xenbus_transaction_end(xbt, 0);
} while (err == -EAGAIN);
@@ -415,6 +423,14 @@ static int connect_rings(struct backend_info *be)
be->netif->dev->features &= ~NETIF_F_IP_CSUM;
}
+ if (xenbus_scanf(XBT_NIL, dev->otherend,
"feature-smart-poll",
+ "%d", &val) < 0)
+ val = 0;
+ if (val)
+ be->netif->smart_poll = 1;
+ else
+ be->netif->smart_poll = 0;
+
/* Map the shared frame, irq etc. */
err = netif_map(be->netif, tx_ring_ref, rx_ring_ref, evtchn);
if (err) {
diff --git a/include/xen/interface/io/ring.h b/include/xen/interface/io/ring.h
index e8cbf43..865dcf0 100644
--- a/include/xen/interface/io/ring.h
+++ b/include/xen/interface/io/ring.h
@@ -73,7 +73,8 @@ union __name##_sring_entry {
\
struct __name##_sring {
\
RING_IDX req_prod, req_event; \
RING_IDX rsp_prod, rsp_event; \
- uint8_t pad[48]; \
+ uint8_t netfront_smartpoll_active;
\
+ uint8_t pad[47]; \
union __name##_sring_entry ring[1]; /* variable-length */ \
}; \
\
--
1.6.3
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Xu, Dongxiao
2009-Oct-01 00:11 UTC
[Xen-devel][PV-ops][PATCH 1/2] VNIF(netback): Using smart polling instead of event notification.
- Netback will not notify netfront until it finds that the netfront has
stopped polling.
- Netback will set a flag in xenstore to indicate whether netback
supports the smart polling feature. If there is only one side
supporting it, the communication mechanism will fall back to default,
and the new feature will not be used. The feature is enabled only
when both sides have the flag set in xenstore.
Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
---
drivers/xen/netback/common.h | 2 ++
drivers/xen/netback/netback.c | 23 +++++++++++++++++++++--
drivers/xen/netback/xenbus.c | 16 ++++++++++++++++
include/xen/interface/io/ring.h | 3 ++-
4 files changed, 41 insertions(+), 3 deletions(-)
diff --git a/drivers/xen/netback/common.h b/drivers/xen/netback/common.h
index 9056be0..b15a7e1 100644
--- a/drivers/xen/netback/common.h
+++ b/drivers/xen/netback/common.h
@@ -76,6 +76,8 @@ struct xen_netif {
/* Set of features that can be turned on in dev->features. */
int features;
+ int smart_poll;
+
/* Internal feature information. */
u8 can_queue:1; /* can queue packets for receiver? */
diff --git a/drivers/xen/netback/netback.c b/drivers/xen/netback/netback.c
index d7d738e..869b6cc 100644
--- a/drivers/xen/netback/netback.c
+++ b/drivers/xen/netback/netback.c
@@ -640,7 +640,8 @@ static void net_rx_action(unsigned long unused)
RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(&netif->rx, ret);
irq = netif->irq;
- if (ret && !rx_notify[irq]) {
+ if (ret && !rx_notify[irq] &&
+ (netif->smart_poll != 1)) {
rx_notify[irq] = 1;
notify_list[notify_nr++] = irq;
}
@@ -650,6 +651,16 @@ static void net_rx_action(unsigned long unused)
!netbk_queue_full(netif))
netif_wake_queue(netif->dev);
+ /* netfront_smartpoll_active indicates whether netfront timer
+ * is active.
+ */
+ if ((netif->smart_poll == 1)) {
+ if (!(netif->rx.sring->netfront_smartpoll_active))
{
+ notify_remote_via_irq(irq);
+ netif->rx.sring->netfront_smartpoll_active
= 1;
+ }
+ }
+
netif_put(netif);
dev_kfree_skb(skb);
npo.meta_cons += nr_frags + 1;
@@ -1471,7 +1482,15 @@ static void make_tx_response(struct xen_netif *netif,
netif->tx.rsp_prod_pvt = ++i;
RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(&netif->tx, notify);
- if (notify)
+ /* netfront_smartpoll_active indicates whether netfront timer is
+ * active.
+ */
+ if ((netif->smart_poll == 1)) {
+ if (!(netif->rx.sring->netfront_smartpoll_active)) {
+ notify_remote_via_irq(netif->irq);
+ netif->rx.sring->netfront_smartpoll_active = 1;
+ }
+ } else if (notify)
notify_remote_via_irq(netif->irq);
}
diff --git a/drivers/xen/netback/xenbus.c b/drivers/xen/netback/xenbus.c
index a492288..3d8ed9a 100644
--- a/drivers/xen/netback/xenbus.c
+++ b/drivers/xen/netback/xenbus.c
@@ -115,6 +115,14 @@ static int netback_probe(struct xenbus_device *dev,
goto abort_transaction;
}
+ /* We support data smart poll mechanism */
+ err = xenbus_printf(xbt, dev->nodename,
+ "feature-smart-poll",
"%d", 1);
+ if (err) {
+ message = "writing feature-smart-poll";
+ goto abort_transaction;
+ }
+
err = xenbus_transaction_end(xbt, 0);
} while (err == -EAGAIN);
@@ -415,6 +423,14 @@ static int connect_rings(struct backend_info *be)
be->netif->dev->features &= ~NETIF_F_IP_CSUM;
}
+ if (xenbus_scanf(XBT_NIL, dev->otherend,
"feature-smart-poll",
+ "%d", &val) < 0)
+ val = 0;
+ if (val)
+ be->netif->smart_poll = 1;
+ else
+ be->netif->smart_poll = 0;
+
/* Map the shared frame, irq etc. */
err = netif_map(be->netif, tx_ring_ref, rx_ring_ref, evtchn);
if (err) {
diff --git a/include/xen/interface/io/ring.h b/include/xen/interface/io/ring.h
index e8cbf43..865dcf0 100644
--- a/include/xen/interface/io/ring.h
+++ b/include/xen/interface/io/ring.h
@@ -73,7 +73,8 @@ union __name##_sring_entry {
\
struct __name##_sring {
\
RING_IDX req_prod, req_event; \
RING_IDX rsp_prod, rsp_event; \
- uint8_t pad[48]; \
+ uint8_t netfront_smartpoll_active;
\
+ uint8_t pad[47]; \
union __name##_sring_entry ring[1]; /* variable-length */ \
}; \
\
--
1.6.3
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel