| Message ID | 20191025135601.19535.55564.stgit@netdev64 |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev,dpdk-latest,v2] netdev-dpdk: add coverage counter to count vhost IRQs | expand |
On 25 Oct 2019, at 15:56, Eelco Chaudron wrote: > When the dpdk vhost library executes an eventfd_write() call, > i.e. waking up the guest, a new callback will be called. > > This patch adds the callback to count the number of > interrupts sent to the VM to track the number of times > interrupts where generated. > > This might be of interest to find out system-calls were > called in the DPDK fast path. > > The coverage counter is called "dpdk_vhost_irqs" and can be > read with: > > $ ovs-appctl coverage/show | grep dpdk_vhost_irqs > dpdk_vhost_irqs 275880.6/sec 129962.683/sec > 6561.7250/sec total: 23684319 > > $ ovs-appctl coverage/read-counter dpdk_vhost_irqs > 23684319 > > Signed-off-by: Eelco Chaudron <echaudro@redhat.com> Any comments on this patchset?
This patch doesn't apply to master, but it applies to dpdk-latest. dpdk-latest needs some rebase and this patch should be rebased after that. Comments inline. Best regards, Ilya Maximets. On 25.10.2019 15:56, Eelco Chaudron wrote: > When the dpdk vhost library executes an eventfd_write() call, > i.e. waking up the guest, a new callback will be called. > > This patch adds the callback to count the number of > interrupts sent to the VM to track the number of times > interrupts where generated. > > This might be of interest to find out system-calls were > called in the DPDK fast path. > > The coverage counter is called "dpdk_vhost_irqs" and can be > read with: > > $ ovs-appctl coverage/show | grep dpdk_vhost_irqs > dpdk_vhost_irqs 275880.6/sec 129962.683/sec 6561.7250/sec total: 23684319 I'm not sure if above line is any valuable here, but it definitely too long for a commit message. > > $ ovs-appctl coverage/read-counter dpdk_vhost_irqs > 23684319 > > Signed-off-by: Eelco Chaudron <echaudro@redhat.com> > --- > lib/netdev-dpdk.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c > index ba92e89..8c6aaeb 100644 > --- a/lib/netdev-dpdk.c > +++ b/lib/netdev-dpdk.c > @@ -46,6 +46,7 @@ > #include "dpdk.h" > #include "dpif-netdev.h" > #include "fatal-signal.h" > +#include "coverage.h" We're trying to keep alphabetical order of includes if possible. > #include "netdev-provider.h" > #include "netdev-vport.h" > #include "odp-util.h" > @@ -72,6 +73,8 @@ enum {VIRTIO_RXQ, VIRTIO_TXQ, VIRTIO_QNUM}; > VLOG_DEFINE_THIS_MODULE(netdev_dpdk); > static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20); > > +COVERAGE_DEFINE(dpdk_vhost_irqs); Now we have 'vhost_tx_contention' and it's better to have similar name here. Maybe 'vhost_notification'? BTW, I'm still not a fan of this concept in general, but It seems to be a part of a public API in DPDK and we'll stick with it for some time at least. > + > #define DPDK_PORT_WATCHDOG_INTERVAL 5 > > #define OVS_CACHE_LINE_SIZE CACHE_LINE_SIZE > @@ -165,6 +168,8 @@ static int new_device(int vid); > static void destroy_device(int vid); > static int vring_state_changed(int vid, uint16_t queue_id, int enable); > static void destroy_connection(int vid); > +static void vhost_guest_notified(int vid); > + > static const struct vhost_device_ops virtio_net_device_ops = > { > .new_device = new_device, > @@ -173,6 +178,7 @@ static const struct vhost_device_ops virtio_net_device_ops = > .features_changed = NULL, > .new_connection = NULL, > .destroy_connection = destroy_connection, > + .guest_notified = vhost_guest_notified, > }; > > enum { DPDK_RING_SIZE = 256 }; > @@ -3746,6 +3752,12 @@ destroy_connection(int vid) > } > } > > +static > +void vhost_guest_notified(int vid OVS_UNUSED) > +{ > + COVERAGE_INC(dpdk_vhost_irqs); > +} > + > /* > * Retrieve the DPDK virtio device ID (vid) associated with a vhostuser > * or vhostuserclient netdev. >
On 11 Nov 2019, at 16:15, Ilya Maximets wrote: > This patch doesn't apply to master, but it applies to dpdk-latest. > dpdk-latest needs some rebase and this patch should be rebased > after that. Yes I know, any idea when this will be? Or how we will handle these DPDK related features? Will re-submit once dpdk-latest is re-based or master has 19.11 DPDK version… > Comments inline. > > Best regards, Ilya Maximets. > > On 25.10.2019 15:56, Eelco Chaudron wrote: >> When the dpdk vhost library executes an eventfd_write() call, >> i.e. waking up the guest, a new callback will be called. >> >> This patch adds the callback to count the number of >> interrupts sent to the VM to track the number of times >> interrupts where generated. >> >> This might be of interest to find out system-calls were >> called in the DPDK fast path. >> >> The coverage counter is called "dpdk_vhost_irqs" and can be >> read with: >> >> $ ovs-appctl coverage/show | grep dpdk_vhost_irqs >> dpdk_vhost_irqs 275880.6/sec 129962.683/sec >> 6561.7250/sec total: 23684319 > > I'm not sure if above line is any valuable here, but it > definitely too long for a commit message. Will remove it… > >> >> $ ovs-appctl coverage/read-counter dpdk_vhost_irqs >> 23684319 >> >> Signed-off-by: Eelco Chaudron <echaudro@redhat.com> >> --- >> lib/netdev-dpdk.c | 12 ++++++++++++ >> 1 file changed, 12 insertions(+) >> >> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c >> index ba92e89..8c6aaeb 100644 >> --- a/lib/netdev-dpdk.c >> +++ b/lib/netdev-dpdk.c >> @@ -46,6 +46,7 @@ >> #include "dpdk.h" >> #include "dpif-netdev.h" >> #include "fatal-signal.h" >> +#include "coverage.h" > > We're trying to keep alphabetical order of includes if possible. I know, wondering why I ignored it ;) Will fix… >> #include "netdev-provider.h" >> #include "netdev-vport.h" >> #include "odp-util.h" >> @@ -72,6 +73,8 @@ enum {VIRTIO_RXQ, VIRTIO_TXQ, VIRTIO_QNUM}; >> VLOG_DEFINE_THIS_MODULE(netdev_dpdk); >> static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20); >> >> +COVERAGE_DEFINE(dpdk_vhost_irqs); > > Now we have 'vhost_tx_contention' and it's better to have > similar name here. Maybe 'vhost_notification'? Sound good to me, will change it to vhost_notification > > BTW, I'm still not a fan of this concept in general, but It seems > to be a part of a public API in DPDK and we'll stick with it for > some time at least. > >> + >> #define DPDK_PORT_WATCHDOG_INTERVAL 5 >> >> #define OVS_CACHE_LINE_SIZE CACHE_LINE_SIZE >> @@ -165,6 +168,8 @@ static int new_device(int vid); >> static void destroy_device(int vid); >> static int vring_state_changed(int vid, uint16_t queue_id, int >> enable); >> static void destroy_connection(int vid); >> +static void vhost_guest_notified(int vid); >> + >> static const struct vhost_device_ops virtio_net_device_ops = >> { >> .new_device = new_device, >> @@ -173,6 +178,7 @@ static const struct vhost_device_ops >> virtio_net_device_ops = >> .features_changed = NULL, >> .new_connection = NULL, >> .destroy_connection = destroy_connection, >> + .guest_notified = vhost_guest_notified, >> }; >> >> enum { DPDK_RING_SIZE = 256 }; >> @@ -3746,6 +3752,12 @@ destroy_connection(int vid) >> } >> } >> >> +static >> +void vhost_guest_notified(int vid OVS_UNUSED) >> +{ >> + COVERAGE_INC(dpdk_vhost_irqs); >> +} >> + >> /* >> * Retrieve the DPDK virtio device ID (vid) associated with a >> vhostuser >> * or vhostuserclient netdev. >>
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index ba92e89..8c6aaeb 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -46,6 +46,7 @@ #include "dpdk.h" #include "dpif-netdev.h" #include "fatal-signal.h" +#include "coverage.h" #include "netdev-provider.h" #include "netdev-vport.h" #include "odp-util.h" @@ -72,6 +73,8 @@ enum {VIRTIO_RXQ, VIRTIO_TXQ, VIRTIO_QNUM}; VLOG_DEFINE_THIS_MODULE(netdev_dpdk); static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20); +COVERAGE_DEFINE(dpdk_vhost_irqs); + #define DPDK_PORT_WATCHDOG_INTERVAL 5 #define OVS_CACHE_LINE_SIZE CACHE_LINE_SIZE @@ -165,6 +168,8 @@ static int new_device(int vid); static void destroy_device(int vid); static int vring_state_changed(int vid, uint16_t queue_id, int enable); static void destroy_connection(int vid); +static void vhost_guest_notified(int vid); + static const struct vhost_device_ops virtio_net_device_ops = { .new_device = new_device, @@ -173,6 +178,7 @@ static const struct vhost_device_ops virtio_net_device_ops = .features_changed = NULL, .new_connection = NULL, .destroy_connection = destroy_connection, + .guest_notified = vhost_guest_notified, }; enum { DPDK_RING_SIZE = 256 }; @@ -3746,6 +3752,12 @@ destroy_connection(int vid) } } +static +void vhost_guest_notified(int vid OVS_UNUSED) +{ + COVERAGE_INC(dpdk_vhost_irqs); +} + /* * Retrieve the DPDK virtio device ID (vid) associated with a vhostuser * or vhostuserclient netdev.
When the dpdk vhost library executes an eventfd_write() call, i.e. waking up the guest, a new callback will be called. This patch adds the callback to count the number of interrupts sent to the VM to track the number of times interrupts where generated. This might be of interest to find out system-calls were called in the DPDK fast path. The coverage counter is called "dpdk_vhost_irqs" and can be read with: $ ovs-appctl coverage/show | grep dpdk_vhost_irqs dpdk_vhost_irqs 275880.6/sec 129962.683/sec 6561.7250/sec total: 23684319 $ ovs-appctl coverage/read-counter dpdk_vhost_irqs 23684319 Signed-off-by: Eelco Chaudron <echaudro@redhat.com> --- lib/netdev-dpdk.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)