| Message ID | 1555407917-2719-3-git-send-email-david.marchand@redhat.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev,v2,1/3] dpif-netdev: only poll enabled vhost queues | expand |
On 16.04.2019 12:45, David Marchand wrote: > Rather than poll all disabled queues and waste some memory for vms that > have been shutdown, we can reconfigure when receiving a destroy > connection notification from the vhost library. > > $ while true; do > ovs-appctl dpif-netdev/pmd-rxq-show |awk ' > /port: / { > tot++; > if ($NF == "disabled") { > dis++; > } > } > END { > print "total: " tot ", enabled: " (tot - dis) > }' > sleep 1 > done > > total: 66, enabled: 66 > total: 6, enabled: 2 > > Note: this patch requires a fix for the vhost library submitted here: > http://patchwork.dpdk.org/patch/52680/ > > Without it, this change will do nothing but have openvswitch complain > that the vhost device is unknown: > > dpdk|INFO|VHOST_CONFIG: vhost peer closed > dpdk|ERR|VHOST_CONFIG: (0) device not found. > > dpdk|INFO|VHOST_CONFIG: vhost peer closed > dpdk|ERR|VHOST_CONFIG: (1) device not found. > > Signed-off-by: David Marchand <david.marchand@redhat.com> > --- > lib/netdev-dpdk.c | 35 ++++++++++++++++++++++++++++++++++- > 1 file changed, 34 insertions(+), 1 deletion(-) > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c > index fc554db..a7fae4f 100644 > --- a/lib/netdev-dpdk.c > +++ b/lib/netdev-dpdk.c > @@ -186,12 +186,15 @@ static const struct rte_eth_conf port_conf = { > 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 const struct vhost_device_ops virtio_net_device_ops = > { > .new_device = new_device, > .destroy_device = destroy_device, > .vring_state_changed = vring_state_changed, > - .features_changed = NULL > + .features_changed = NULL, > + .new_connection = NULL, > + .destroy_connection = destroy_connection, > }; > > enum { DPDK_RING_SIZE = 256 }; > @@ -3661,6 +3664,36 @@ vring_state_changed(int vid, uint16_t queue_id, int enable) > return 0; > } > > +static void > +destroy_connection(int vid) > +{ > + struct netdev_dpdk *dev; > + char ifname[IF_NAME_SZ]; > + > + rte_vhost_get_ifname(vid, ifname, sizeof ifname); > + > + ovs_mutex_lock(&dpdk_mutex); > + /* Add device to the vhost port with the same name as that passed down. */ This comment is a leftover from the 'new_device'. It could be dropped. > + LIST_FOR_EACH (dev, list_node, &dpdk_list) { > + ovs_mutex_lock(&dev->mutex); > + if (nullable_string_is_equal(ifname, dev->vhost_id)) { > + uint32_t qp_num = NR_QUEUE; > + It might be good to check if device is destroyed here and log an error otherwise. > + /* Restore the number of queue pairs to default. */ > + if (dev->requested_n_txq != qp_num > + || dev->requested_n_rxq != qp_num) { > + dev->requested_n_rxq = qp_num; > + dev->requested_n_txq = qp_num; > + netdev_request_reconfigure(&dev->up); > + } > + ovs_mutex_unlock(&dev->mutex); > + break; > + } > + ovs_mutex_unlock(&dev->mutex); > + } > + ovs_mutex_unlock(&dpdk_mutex); > +} > + > /* > * Retrieve the DPDK virtio device ID (vid) associated with a vhostuser > * or vhostuserclient netdev. >
On Tue, Apr 16, 2019 at 4:01 PM Ilya Maximets <i.maximets@samsung.com> wrote: > On 16.04.2019 12:45, David Marchand wrote: > > Rather than poll all disabled queues and waste some memory for vms that > > have been shutdown, we can reconfigure when receiving a destroy > > connection notification from the vhost library. > > > > $ while true; do > > ovs-appctl dpif-netdev/pmd-rxq-show |awk ' > > /port: / { > > tot++; > > if ($NF == "disabled") { > > dis++; > > } > > } > > END { > > print "total: " tot ", enabled: " (tot - dis) > > }' > > sleep 1 > > done > > > > total: 66, enabled: 66 > > total: 6, enabled: 2 > > > > Note: this patch requires a fix for the vhost library submitted here: > > http://patchwork.dpdk.org/patch/52680/ > > > > Without it, this change will do nothing but have openvswitch complain > > that the vhost device is unknown: > > > > dpdk|INFO|VHOST_CONFIG: vhost peer closed > > dpdk|ERR|VHOST_CONFIG: (0) device not found. > > > > dpdk|INFO|VHOST_CONFIG: vhost peer closed > > dpdk|ERR|VHOST_CONFIG: (1) device not found. > > > > Signed-off-by: David Marchand <david.marchand@redhat.com> > > --- > > lib/netdev-dpdk.c | 35 ++++++++++++++++++++++++++++++++++- > > 1 file changed, 34 insertions(+), 1 deletion(-) > > > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c > > index fc554db..a7fae4f 100644 > > --- a/lib/netdev-dpdk.c > > +++ b/lib/netdev-dpdk.c > > @@ -186,12 +186,15 @@ static const struct rte_eth_conf port_conf = { > > 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 const struct vhost_device_ops virtio_net_device_ops = > > { > > .new_device = new_device, > > .destroy_device = destroy_device, > > .vring_state_changed = vring_state_changed, > > - .features_changed = NULL > > + .features_changed = NULL, > > + .new_connection = NULL, > > + .destroy_connection = destroy_connection, > > }; > > > > enum { DPDK_RING_SIZE = 256 }; > > @@ -3661,6 +3664,36 @@ vring_state_changed(int vid, uint16_t queue_id, > int enable) > > return 0; > > } > > > > +static void > > +destroy_connection(int vid) > > +{ > > + struct netdev_dpdk *dev; > > + char ifname[IF_NAME_SZ]; > > + > > + rte_vhost_get_ifname(vid, ifname, sizeof ifname); > > + > > + ovs_mutex_lock(&dpdk_mutex); > > + /* Add device to the vhost port with the same name as that passed > down. */ > > This comment is a leftover from the 'new_device'. It could be dropped. > Yep. > > + LIST_FOR_EACH (dev, list_node, &dpdk_list) { > > + ovs_mutex_lock(&dev->mutex); > > + if (nullable_string_is_equal(ifname, dev->vhost_id)) { > > + uint32_t qp_num = NR_QUEUE; > > + > > It might be good to check if device is destroyed here and log an error > otherwise. > So that we have a log similar to what we have in destroy_device() ?
On 17.04.2019 11:37, David Marchand wrote: > > > > On Tue, Apr 16, 2019 at 4:01 PM Ilya Maximets <i.maximets@samsung.com <mailto:i.maximets@samsung.com>> wrote: > > On 16.04.2019 12:45, David Marchand wrote: > > Rather than poll all disabled queues and waste some memory for vms that > > have been shutdown, we can reconfigure when receiving a destroy > > connection notification from the vhost library. > > > > $ while true; do > > ovs-appctl dpif-netdev/pmd-rxq-show |awk ' > > /port: / { > > tot++; > > if ($NF == "disabled") { > > dis++; > > } > > } > > END { > > print "total: " tot ", enabled: " (tot - dis) > > }' > > sleep 1 > > done > > > > total: 66, enabled: 66 > > total: 6, enabled: 2 > > > > Note: this patch requires a fix for the vhost library submitted here: > > http://patchwork.dpdk.org/patch/52680/ > > > > Without it, this change will do nothing but have openvswitch complain > > that the vhost device is unknown: > > > > dpdk|INFO|VHOST_CONFIG: vhost peer closed > > dpdk|ERR|VHOST_CONFIG: (0) device not found. > > > > dpdk|INFO|VHOST_CONFIG: vhost peer closed > > dpdk|ERR|VHOST_CONFIG: (1) device not found. > > > > Signed-off-by: David Marchand <david.marchand@redhat.com <mailto:david.marchand@redhat.com>> > > --- > > lib/netdev-dpdk.c | 35 ++++++++++++++++++++++++++++++++++- > > 1 file changed, 34 insertions(+), 1 deletion(-) > > > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c > > index fc554db..a7fae4f 100644 > > --- a/lib/netdev-dpdk.c > > +++ b/lib/netdev-dpdk.c > > @@ -186,12 +186,15 @@ static const struct rte_eth_conf port_conf = { > > 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 const struct vhost_device_ops virtio_net_device_ops = > > { > > .new_device = new_device, > > .destroy_device = destroy_device, > > .vring_state_changed = vring_state_changed, > > - .features_changed = NULL > > + .features_changed = NULL, > > + .new_connection = NULL, > > + .destroy_connection = destroy_connection, > > }; > > > > enum { DPDK_RING_SIZE = 256 }; > > @@ -3661,6 +3664,36 @@ vring_state_changed(int vid, uint16_t queue_id, int enable) > > return 0; > > } > > > > +static void > > +destroy_connection(int vid) > > +{ > > + struct netdev_dpdk *dev; > > + char ifname[IF_NAME_SZ]; > > + > > + rte_vhost_get_ifname(vid, ifname, sizeof ifname); > > + > > + ovs_mutex_lock(&dpdk_mutex); > > + /* Add device to the vhost port with the same name as that passed down. */ > > This comment is a leftover from the 'new_device'. It could be dropped. > > > Yep. > > > > > + LIST_FOR_EACH (dev, list_node, &dpdk_list) { > > + ovs_mutex_lock(&dev->mutex); > > + if (nullable_string_is_equal(ifname, dev->vhost_id)) { > > + uint32_t qp_num = NR_QUEUE; > > + > > It might be good to check if device is destroyed here and log an error > otherwise. > > > So that we have a log similar to what we have in destroy_device() ? Not sure if I understand the question correctly. I meant something like: if (netdev_dpdk_get_vid(dev) >= 0) { VLOG_ERR(Connection on socket '%s' destroyed while vhost device still attached.", dev->vhost_id); } But it's probably a good idea to log an error if we didn't find any matching device in 'dpdk_list' too. > > > -- > David Marchand
On 16/04/2019 10:45, David Marchand wrote: > Rather than poll all disabled queues and waste some memory for vms that > have been shutdown, we can reconfigure when receiving a destroy > connection notification from the vhost library. > > $ while true; do > ovs-appctl dpif-netdev/pmd-rxq-show |awk ' > /port: / { > tot++; > if ($NF == "disabled") { > dis++; > } > } > END { > print "total: " tot ", enabled: " (tot - dis) > }' > sleep 1 > done > > total: 66, enabled: 66 > total: 6, enabled: 2 > > Note: this patch requires a fix for the vhost library submitted here: > http://patchwork.dpdk.org/patch/52680/ > > Without it, this change will do nothing but have openvswitch complain > that the vhost device is unknown: > > dpdk|INFO|VHOST_CONFIG: vhost peer closed > dpdk|ERR|VHOST_CONFIG: (0) device not found. > > dpdk|INFO|VHOST_CONFIG: vhost peer closed > dpdk|ERR|VHOST_CONFIG: (1) device not found. > > Signed-off-by: David Marchand <david.marchand@redhat.com> I think this probably shouldn't be merged until OVS is using a version of DPDK with the linked patch because it is introducing errors in the logs which can be alarming for a user. The DPDK fix should be part of DPDK 18.11.2.
On Wed, Apr 17, 2019 at 12:21 PM Ilya Maximets <i.maximets@samsung.com> wrote: > On 17.04.2019 11:37, David Marchand wrote: > > On Tue, Apr 16, 2019 at 4:01 PM Ilya Maximets <i.maximets@samsung.com > <mailto:i.maximets@samsung.com>> wrote: > > > + LIST_FOR_EACH (dev, list_node, &dpdk_list) { > > > + ovs_mutex_lock(&dev->mutex); > > > + if (nullable_string_is_equal(ifname, dev->vhost_id)) { > > > + uint32_t qp_num = NR_QUEUE; > > > + > > > > It might be good to check if device is destroyed here and log an > error > > otherwise. > > > > > > So that we have a log similar to what we have in destroy_device() ? > > Not sure if I understand the question correctly. I meant something like: > > if (netdev_dpdk_get_vid(dev) >= 0) { > VLOG_ERR(Connection on socket '%s' destroyed while vhost device still > attached.", > dev->vhost_id); > } > > But it's probably a good idea to log an error if we didn't find any > matching > device in 'dpdk_list' too. > Ok, I had the latter in mind, thanks for clarifying the check on netdev_dpdk_get_vid. Will add both.
On Wed, Apr 17, 2019 at 4:27 PM Kevin Traynor <ktraynor@redhat.com> wrote: > On 16/04/2019 10:45, David Marchand wrote: > > > Note: this patch requires a fix for the vhost library submitted here: > > http://patchwork.dpdk.org/patch/52680/ > > > > Without it, this change will do nothing but have openvswitch complain > > that the vhost device is unknown: > > > > dpdk|INFO|VHOST_CONFIG: vhost peer closed > > dpdk|ERR|VHOST_CONFIG: (0) device not found. > > > > dpdk|INFO|VHOST_CONFIG: vhost peer closed > > dpdk|ERR|VHOST_CONFIG: (1) device not found. > > > > Signed-off-by: David Marchand <david.marchand@redhat.com> > > I think this probably shouldn't be merged until OVS is using a version > of DPDK with the linked patch because it is introducing errors in the > logs which can be alarming for a user. The DPDK fix should be part of > DPDK 18.11.2. > It sounds sane yes. Now, 18.11.2 is not ready yet (no pressure Kevin ;-)) as we just got 18.11.1. Ilya, if we go that way, what do you think of considering the first patch for merge now and the others two fixes for when 18.11.2 is ready ?
On 18.04.2019 14:26, David Marchand wrote: > On Wed, Apr 17, 2019 at 4:27 PM Kevin Traynor <ktraynor@redhat.com <mailto:ktraynor@redhat.com>> wrote: > > On 16/04/2019 10:45, David Marchand wrote: > > > Note: this patch requires a fix for the vhost library submitted here: > > http://patchwork.dpdk.org/patch/52680/ > > > > Without it, this change will do nothing but have openvswitch complain > > that the vhost device is unknown: > > > > dpdk|INFO|VHOST_CONFIG: vhost peer closed > > dpdk|ERR|VHOST_CONFIG: (0) device not found. > > > > dpdk|INFO|VHOST_CONFIG: vhost peer closed > > dpdk|ERR|VHOST_CONFIG: (1) device not found. > > > > Signed-off-by: David Marchand <david.marchand@redhat.com <mailto:david.marchand@redhat.com>> > > I think this probably shouldn't be merged until OVS is using a version > of DPDK with the linked patch because it is introducing errors in the > logs which can be alarming for a user. The DPDK fix should be part of > DPDK 18.11.2. > > > It sounds sane yes. > Now, 18.11.2 is not ready yet (no pressure Kevin ;-)) as we just got 18.11.1. > > Ilya, if we go that way, what do you think of considering the first patch for merge now and the others two fixes for when 18.11.2 is ready ? I think, we can go with first two patches now and merge the last one when the 18.11.2 ready. Second patch should not make any harm as all modern drivers has F_MQ support and we'll not waste much time on polling disabled queues. Anyway, polling of disabled queues better than guest-controllable reconfigurations. We also need to think about backporting the second patch. > -- > David Marchand
On Thu, Apr 18, 2019 at 1:51 PM Ilya Maximets <i.maximets@samsung.com> wrote: > > > On 18.04.2019 14:26, David Marchand wrote: > > On Wed, Apr 17, 2019 at 4:27 PM Kevin Traynor <ktraynor@redhat.com > <mailto:ktraynor@redhat.com>> wrote: > > > > On 16/04/2019 10:45, David Marchand wrote: > > > > > Note: this patch requires a fix for the vhost library submitted > here: > > > http://patchwork.dpdk.org/patch/52680/ > > > > > > Without it, this change will do nothing but have openvswitch > complain > > > that the vhost device is unknown: > > > > > > dpdk|INFO|VHOST_CONFIG: vhost peer closed > > > dpdk|ERR|VHOST_CONFIG: (0) device not found. > > > > > > dpdk|INFO|VHOST_CONFIG: vhost peer closed > > > dpdk|ERR|VHOST_CONFIG: (1) device not found. > > > > > > Signed-off-by: David Marchand <david.marchand@redhat.com <mailto: > david.marchand@redhat.com>> > > > > I think this probably shouldn't be merged until OVS is using a > version > > of DPDK with the linked patch because it is introducing errors in the > > logs which can be alarming for a user. The DPDK fix should be part of > > DPDK 18.11.2. > > > > > > It sounds sane yes. > > Now, 18.11.2 is not ready yet (no pressure Kevin ;-)) as we just got > 18.11.1. > > > > Ilya, if we go that way, what do you think of considering the first > patch for merge now and the others two fixes for when 18.11.2 is ready ? > > I think, we can go with first two patches now and merge the last one when > the 18.11.2 ready. Second patch should not make any harm as all modern > drivers has F_MQ support and we'll not waste much time on polling disabled > queues. Anyway, polling of disabled queues better than guest-controllable > reconfigurations. We also need to think about backporting the second patch. > Ok, how should I proceed ? Can I still submit the 3 patches together, to finish handling the last comments ?
On 18.04.2019 17:05, David Marchand wrote: > On Thu, Apr 18, 2019 at 1:51 PM Ilya Maximets <i.maximets@samsung.com <mailto:i.maximets@samsung.com>> wrote: > > > > On 18.04.2019 14:26, David Marchand wrote: > > On Wed, Apr 17, 2019 at 4:27 PM Kevin Traynor <ktraynor@redhat.com <mailto:ktraynor@redhat.com> <mailto:ktraynor@redhat.com <mailto:ktraynor@redhat.com>>> wrote: > > > > On 16/04/2019 10:45, David Marchand wrote: > > > > > Note: this patch requires a fix for the vhost library submitted here: > > > http://patchwork.dpdk.org/patch/52680/ > > > > > > Without it, this change will do nothing but have openvswitch complain > > > that the vhost device is unknown: > > > > > > dpdk|INFO|VHOST_CONFIG: vhost peer closed > > > dpdk|ERR|VHOST_CONFIG: (0) device not found. > > > > > > dpdk|INFO|VHOST_CONFIG: vhost peer closed > > > dpdk|ERR|VHOST_CONFIG: (1) device not found. > > > > > > Signed-off-by: David Marchand <david.marchand@redhat.com <mailto:david.marchand@redhat.com> <mailto:david.marchand@redhat.com <mailto:david.marchand@redhat.com>>> > > > > I think this probably shouldn't be merged until OVS is using a version > > of DPDK with the linked patch because it is introducing errors in the > > logs which can be alarming for a user. The DPDK fix should be part of > > DPDK 18.11.2. > > > > > > It sounds sane yes. > > Now, 18.11.2 is not ready yet (no pressure Kevin ;-)) as we just got 18.11.1. > > > > Ilya, if we go that way, what do you think of considering the first patch for merge now and the others two fixes for when 18.11.2 is ready ? > > I think, we can go with first two patches now and merge the last one when > the 18.11.2 ready. Second patch should not make any harm as all modern > drivers has F_MQ support and we'll not waste much time on polling disabled > queues. Anyway, polling of disabled queues better than guest-controllable > > reconfigurations. We also need to think about backporting the second patch. > > > Ok, how should I proceed ? > Can I still submit the 3 patches together, to finish handling the last comments ? Yes, I think so. You may just add a comment under the cut line that this patch should be applied only after upgrade to DPDK 18.11.2. You may also move the 'Note' from the commit message there, it'll not be needed in commit message after all. Ian, is it OK for you? > -- > David Marchand
On 4/18/2019 3:44 PM, Ilya Maximets wrote: > On 18.04.2019 17:05, David Marchand wrote: >> On Thu, Apr 18, 2019 at 1:51 PM Ilya Maximets <i.maximets@samsung.com <mailto:i.maximets@samsung.com>> wrote: >> >> >> >> On 18.04.2019 14:26, David Marchand wrote: >> > On Wed, Apr 17, 2019 at 4:27 PM Kevin Traynor <ktraynor@redhat.com <mailto:ktraynor@redhat.com> <mailto:ktraynor@redhat.com <mailto:ktraynor@redhat.com>>> wrote: >> > >> > On 16/04/2019 10:45, David Marchand wrote: >> > >> > > Note: this patch requires a fix for the vhost library submitted here: >> > > http://patchwork.dpdk.org/patch/52680/ >> > > >> > > Without it, this change will do nothing but have openvswitch complain >> > > that the vhost device is unknown: >> > > >> > > dpdk|INFO|VHOST_CONFIG: vhost peer closed >> > > dpdk|ERR|VHOST_CONFIG: (0) device not found. >> > > >> > > dpdk|INFO|VHOST_CONFIG: vhost peer closed >> > > dpdk|ERR|VHOST_CONFIG: (1) device not found. >> > > >> > > Signed-off-by: David Marchand <david.marchand@redhat.com <mailto:david.marchand@redhat.com> <mailto:david.marchand@redhat.com <mailto:david.marchand@redhat.com>>> >> > >> > I think this probably shouldn't be merged until OVS is using a version >> > of DPDK with the linked patch because it is introducing errors in the >> > logs which can be alarming for a user. The DPDK fix should be part of >> > DPDK 18.11.2. >> > >> > >> > It sounds sane yes. >> > Now, 18.11.2 is not ready yet (no pressure Kevin ;-)) as we just got 18.11.1. >> > >> > Ilya, if we go that way, what do you think of considering the first patch for merge now and the others two fixes for when 18.11.2 is ready ? >> >> I think, we can go with first two patches now and merge the last one when >> the 18.11.2 ready. Second patch should not make any harm as all modern >> drivers has F_MQ support and we'll not waste much time on polling disabled >> queues. Anyway, polling of disabled queues better than guest-controllable >> >> reconfigurations. We also need to think about backporting the second patch. >> >> >> Ok, how should I proceed ? >> Can I still submit the 3 patches together, to finish handling the last comments ? > > Yes, I think so. > You may just add a comment under the cut line that this patch should be applied > only after upgrade to DPDK 18.11.2. You may also move the 'Note' from the commit > message there, it'll not be needed in commit message after all. > > Ian, is it OK for you? Hi guys, I've applied patches 1 and 2 of the series as there's no requirement for 18.11.2 there. I've also sent a v2 of 18.11.2 patch, just waiting for review before pushing. https://mail.openvswitch.org/pipermail/ovs-dev/2019-June/360143.html Once that's in I think David you can re-spin this patch and we can apply then. Regards Ian > >> -- >> David Marchand
Hello Ian, On Wed, Jun 26, 2019 at 11:10 PM Ian Stokes <ian.stokes@intel.com> wrote: > On 4/18/2019 3:44 PM, Ilya Maximets wrote: > > On 18.04.2019 17:05, David Marchand wrote: > >> Ok, how should I proceed ? > >> Can I still submit the 3 patches together, to finish handling the last > comments ? > > > > Yes, I think so. > > You may just add a comment under the cut line that this patch should be > applied > > only after upgrade to DPDK 18.11.2. You may also move the 'Note' from > the commit > > message there, it'll not be needed in commit message after all. > > > > Ian, is it OK for you? > > Hi guys, I've applied patches 1 and 2 of the series as there's no > requirement for 18.11.2 there. > > I've also sent a v2 of 18.11.2 patch, just waiting for review before > pushing. > > https://mail.openvswitch.org/pipermail/ovs-dev/2019-June/360143.html > > Once that's in I think David you can re-spin this patch and we can apply > then. > Thanks, I am preparing a respin for this last patch on top of yours.
On 6/27/2019 8:26 AM, David Marchand wrote: > Hello Ian, > > On Wed, Jun 26, 2019 at 11:10 PM Ian Stokes <ian.stokes@intel.com > <mailto:ian.stokes@intel.com>> wrote: > > On 4/18/2019 3:44 PM, Ilya Maximets wrote: > > On 18.04.2019 17:05, David Marchand wrote: > >> Ok, how should I proceed ? > >> Can I still submit the 3 patches together, to finish handling > the last comments ? > > > > Yes, I think so. > > You may just add a comment under the cut line that this patch > should be applied > > only after upgrade to DPDK 18.11.2. You may also move the 'Note' > from the commit > > message there, it'll not be needed in commit message after all. > > > > Ian, is it OK for you? > > Hi guys, I've applied patches 1 and 2 of the series as there's no > requirement for 18.11.2 there. > > I've also sent a v2 of 18.11.2 patch, just waiting for review before > pushing. > > https://mail.openvswitch.org/pipermail/ovs-dev/2019-June/360143.html > > Once that's in I think David you can re-spin this patch and we can > apply > then. > > > Thanks, I am preparing a respin for this last patch on top of yours. > Great, I've just pushed support for 18.11.2 to master & branch-2.11, so you can rebase and re-send when suits. Thanks Ian > > -- > David Marchand
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index fc554db..a7fae4f 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -186,12 +186,15 @@ static const struct rte_eth_conf port_conf = { 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 const struct vhost_device_ops virtio_net_device_ops = { .new_device = new_device, .destroy_device = destroy_device, .vring_state_changed = vring_state_changed, - .features_changed = NULL + .features_changed = NULL, + .new_connection = NULL, + .destroy_connection = destroy_connection, }; enum { DPDK_RING_SIZE = 256 }; @@ -3661,6 +3664,36 @@ vring_state_changed(int vid, uint16_t queue_id, int enable) return 0; } +static void +destroy_connection(int vid) +{ + struct netdev_dpdk *dev; + char ifname[IF_NAME_SZ]; + + rte_vhost_get_ifname(vid, ifname, sizeof ifname); + + ovs_mutex_lock(&dpdk_mutex); + /* Add device to the vhost port with the same name as that passed down. */ + LIST_FOR_EACH (dev, list_node, &dpdk_list) { + ovs_mutex_lock(&dev->mutex); + if (nullable_string_is_equal(ifname, dev->vhost_id)) { + uint32_t qp_num = NR_QUEUE; + + /* Restore the number of queue pairs to default. */ + if (dev->requested_n_txq != qp_num + || dev->requested_n_rxq != qp_num) { + dev->requested_n_rxq = qp_num; + dev->requested_n_txq = qp_num; + netdev_request_reconfigure(&dev->up); + } + ovs_mutex_unlock(&dev->mutex); + break; + } + ovs_mutex_unlock(&dev->mutex); + } + ovs_mutex_unlock(&dpdk_mutex); +} + /* * Retrieve the DPDK virtio device ID (vid) associated with a vhostuser * or vhostuserclient netdev.
Rather than poll all disabled queues and waste some memory for vms that have been shutdown, we can reconfigure when receiving a destroy connection notification from the vhost library. $ while true; do ovs-appctl dpif-netdev/pmd-rxq-show |awk ' /port: / { tot++; if ($NF == "disabled") { dis++; } } END { print "total: " tot ", enabled: " (tot - dis) }' sleep 1 done total: 66, enabled: 66 total: 6, enabled: 2 Note: this patch requires a fix for the vhost library submitted here: http://patchwork.dpdk.org/patch/52680/ Without it, this change will do nothing but have openvswitch complain that the vhost device is unknown: dpdk|INFO|VHOST_CONFIG: vhost peer closed dpdk|ERR|VHOST_CONFIG: (0) device not found. dpdk|INFO|VHOST_CONFIG: vhost peer closed dpdk|ERR|VHOST_CONFIG: (1) device not found. Signed-off-by: David Marchand <david.marchand@redhat.com> --- lib/netdev-dpdk.c | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-)