Re: [ovs-dev] [PATCH 1/3] netdev-dpdk: Fix and document vhost tx retries.
On 25/06/2019 11:17, Ilya Maximets wrote: > On 25.06.2019 12:32, Kevin Traynor wrote: >> On 24/06/2019 22:40, Flavio Leitner wrote: >>> On Fri, Jun 21, 2019 at 02:41:56PM +0100, Kevin Traynor wrote: Fix minor issue of one additional retry and add documentation about vhost tx retries and ways to reduce/remove them. Signed-off-by: Kevin Traynor --- There is a checkpatch warning that one of the libvirt lines in docs is a few characters too long. It is similar for some others in the file and I think it makes sense to leave it as is but can wrap if required. --- Documentation/topics/dpdk/vhost-user.rst | 31 lib/netdev-dpdk.c| 2 +- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/Documentation/topics/dpdk/vhost-user.rst b/Documentation/topics/dpdk/vhost-user.rst index f7b4b338e..d1682fd05 100644 --- a/Documentation/topics/dpdk/vhost-user.rst +++ b/Documentation/topics/dpdk/vhost-user.rst @@ -76,4 +76,35 @@ mode ports require QEMU version 2.7. Ports of type vhost-user are currently deprecated and will be removed in a future release. +vhost tx retries + + +When sending a batch of packets (max is 32) to a vhost-user or >>> >>> Maybe use the actual define name (NETDEV_MAX_BURST) since it might change? >>> >> >> I agree the doc could go stale if it ever changed. OTOH, I want a user >> to be able to read the doc part as a standalone without having to search >> the code. I think best to say NETDEV_MAX_BURST and add it's currently >> 32. If it ever changed in the code, an author or reviewer would find the >> comment through a search and be able to update. >> +vhost-user-client interface, it may happen that some but not all of the packets +in the batch are able to be sent to the guest. This is often because there is +not enough free descriptors in the virtqueue for all the packets to be sent. +In this case there will be a retry, with a default maximum of 8 occurring. If +at any time no packets can be sent, it may mean the guest is not accepting +packets, so there are no (more) retries. + +Tx Retries may be reduced or removed by some external configuration, such >>> >>> s/removed/even avoided/ ? >>> >> >> That sounds better, will change it. >> +as increasing the virtqueue size through the ``rx_queue_size`` parameter in +libvirt:: + + + + + + + >>> function='0x0'/> + >>> >>> Do we need to worry about the qemu version? >>> >> >> hmm, yeah, it was definitely not always available. I will check the >> minimum version and add. >> + +The guest application will also need need to provide enough descriptors. For +example with ``testpmd`` the command line argument can be used:: + + --rxd=1024 --txd=1024 + +The guest should also have sufficient cores dedicated for consuming and +processing packets at the required rate. + .. _dpdk-vhost-user: diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 4856c56aa..0f3b9c6f4 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -2353,5 +2353,5 @@ __netdev_dpdk_vhost_send(struct netdev *netdev, int qid, break; } -} while (cnt && (retries++ <= VHOST_ENQ_RETRY_NUM)); +} while (cnt && (retries++ < VHOST_ENQ_RETRY_NUM)); >>> >>> You're removing the packet's last hope to reach home, are you sure? :-) >> >> pop-up dialog box coming in v2 :-) > > Maybe we could just increase VHOST_ENQ_RETRY_NUM by 1 to avoid any > functional changes in this patch? > Just wasn't sure if it was worth it's own patch for something so minor but I can put it in a separate patch and leave this patch to be pure docs for v2. I checked last week and it looks like the additional retry crept in at some point with another change. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 1/3] netdev-dpdk: Fix and document vhost tx retries.
On 25.06.2019 12:32, Kevin Traynor wrote: > On 24/06/2019 22:40, Flavio Leitner wrote: >> On Fri, Jun 21, 2019 at 02:41:56PM +0100, Kevin Traynor wrote: >>> Fix minor issue of one additional retry and add >>> documentation about vhost tx retries and ways to >>> reduce/remove them. >>> >>> Signed-off-by: Kevin Traynor >>> >>> --- >>> There is a checkpatch warning that one of the libvirt >>> lines in docs is a few characters too long. It is similar >>> for some others in the file and I think it makes sense to >>> leave it as is but can wrap if required. >>> --- >>> Documentation/topics/dpdk/vhost-user.rst | 31 >>> lib/netdev-dpdk.c| 2 +- >>> 2 files changed, 32 insertions(+), 1 deletion(-) >>> >>> diff --git a/Documentation/topics/dpdk/vhost-user.rst >>> b/Documentation/topics/dpdk/vhost-user.rst >>> index f7b4b338e..d1682fd05 100644 >>> --- a/Documentation/topics/dpdk/vhost-user.rst >>> +++ b/Documentation/topics/dpdk/vhost-user.rst >>> @@ -76,4 +76,35 @@ mode ports require QEMU version 2.7. Ports of type >>> vhost-user are currently >>> deprecated and will be removed in a future release. >>> >>> +vhost tx retries >>> + >>> + >>> +When sending a batch of packets (max is 32) to a vhost-user or >> >> Maybe use the actual define name (NETDEV_MAX_BURST) since it might change? >> > > I agree the doc could go stale if it ever changed. OTOH, I want a user > to be able to read the doc part as a standalone without having to search > the code. I think best to say NETDEV_MAX_BURST and add it's currently > 32. If it ever changed in the code, an author or reviewer would find the > comment through a search and be able to update. > >>> +vhost-user-client interface, it may happen that some but not all of the >>> packets >>> +in the batch are able to be sent to the guest. This is often because there >>> is >>> +not enough free descriptors in the virtqueue for all the packets to be >>> sent. >>> +In this case there will be a retry, with a default maximum of 8 occurring. >>> If >>> +at any time no packets can be sent, it may mean the guest is not accepting >>> +packets, so there are no (more) retries. >>> + >>> +Tx Retries may be reduced or removed by some external configuration, such >> >> s/removed/even avoided/ ? >> > > That sounds better, will change it. > >>> +as increasing the virtqueue size through the ``rx_queue_size`` parameter in >>> +libvirt:: >>> + >>> + >>> + >>> + >>> + >>> + >>> + >> function='0x0'/> >>> + >> >> Do we need to worry about the qemu version? >> > > hmm, yeah, it was definitely not always available. I will check the > minimum version and add. > >>> + >>> +The guest application will also need need to provide enough descriptors. >>> For >>> +example with ``testpmd`` the command line argument can be used:: >>> + >>> + --rxd=1024 --txd=1024 >>> + >>> +The guest should also have sufficient cores dedicated for consuming and >>> +processing packets at the required rate. >>> + >>> .. _dpdk-vhost-user: >>> >>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c >>> index 4856c56aa..0f3b9c6f4 100644 >>> --- a/lib/netdev-dpdk.c >>> +++ b/lib/netdev-dpdk.c >>> @@ -2353,5 +2353,5 @@ __netdev_dpdk_vhost_send(struct netdev *netdev, int >>> qid, >>> break; >>> } >>> -} while (cnt && (retries++ <= VHOST_ENQ_RETRY_NUM)); >>> +} while (cnt && (retries++ < VHOST_ENQ_RETRY_NUM)); >> >> You're removing the packet's last hope to reach home, are you sure? :-) > > pop-up dialog box coming in v2 :-) Maybe we could just increase VHOST_ENQ_RETRY_NUM by 1 to avoid any functional changes in this patch? ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 1/3] netdev-dpdk: Fix and document vhost tx retries.
On 24/06/2019 22:40, Flavio Leitner wrote: > On Fri, Jun 21, 2019 at 02:41:56PM +0100, Kevin Traynor wrote: >> Fix minor issue of one additional retry and add >> documentation about vhost tx retries and ways to >> reduce/remove them. >> >> Signed-off-by: Kevin Traynor >> >> --- >> There is a checkpatch warning that one of the libvirt >> lines in docs is a few characters too long. It is similar >> for some others in the file and I think it makes sense to >> leave it as is but can wrap if required. >> --- >> Documentation/topics/dpdk/vhost-user.rst | 31 >> lib/netdev-dpdk.c| 2 +- >> 2 files changed, 32 insertions(+), 1 deletion(-) >> >> diff --git a/Documentation/topics/dpdk/vhost-user.rst >> b/Documentation/topics/dpdk/vhost-user.rst >> index f7b4b338e..d1682fd05 100644 >> --- a/Documentation/topics/dpdk/vhost-user.rst >> +++ b/Documentation/topics/dpdk/vhost-user.rst >> @@ -76,4 +76,35 @@ mode ports require QEMU version 2.7. Ports of type >> vhost-user are currently >> deprecated and will be removed in a future release. >> >> +vhost tx retries >> + >> + >> +When sending a batch of packets (max is 32) to a vhost-user or > > Maybe use the actual define name (NETDEV_MAX_BURST) since it might change? > I agree the doc could go stale if it ever changed. OTOH, I want a user to be able to read the doc part as a standalone without having to search the code. I think best to say NETDEV_MAX_BURST and add it's currently 32. If it ever changed in the code, an author or reviewer would find the comment through a search and be able to update. >> +vhost-user-client interface, it may happen that some but not all of the >> packets >> +in the batch are able to be sent to the guest. This is often because there >> is >> +not enough free descriptors in the virtqueue for all the packets to be sent. >> +In this case there will be a retry, with a default maximum of 8 occurring. >> If >> +at any time no packets can be sent, it may mean the guest is not accepting >> +packets, so there are no (more) retries. >> + >> +Tx Retries may be reduced or removed by some external configuration, such > > s/removed/even avoided/ ? > That sounds better, will change it. >> +as increasing the virtqueue size through the ``rx_queue_size`` parameter in >> +libvirt:: >> + >> + >> + >> + >> + >> + >> + > function='0x0'/> >> + > > Do we need to worry about the qemu version? > hmm, yeah, it was definitely not always available. I will check the minimum version and add. >> + >> +The guest application will also need need to provide enough descriptors. For >> +example with ``testpmd`` the command line argument can be used:: >> + >> + --rxd=1024 --txd=1024 >> + >> +The guest should also have sufficient cores dedicated for consuming and >> +processing packets at the required rate. >> + >> .. _dpdk-vhost-user: >> >> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c >> index 4856c56aa..0f3b9c6f4 100644 >> --- a/lib/netdev-dpdk.c >> +++ b/lib/netdev-dpdk.c >> @@ -2353,5 +2353,5 @@ __netdev_dpdk_vhost_send(struct netdev *netdev, int >> qid, >> break; >> } >> -} while (cnt && (retries++ <= VHOST_ENQ_RETRY_NUM)); >> +} while (cnt && (retries++ < VHOST_ENQ_RETRY_NUM)); > > You're removing the packet's last hope to reach home, are you sure? :-) pop-up dialog box coming in v2 :-) thanks for review, Kevin. > fbl > >> >> rte_spinlock_unlock(>tx_q[qid].tx_lock); >> -- >> 2.20.1 >> >> ___ >> dev mailing list >> d...@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 1/3] netdev-dpdk: Fix and document vhost tx retries.
On 21 Jun 2019, at 15:41, Kevin Traynor wrote: > Fix minor issue of one additional retry and add > documentation about vhost tx retries and ways to > reduce/remove them. > > Signed-off-by: Kevin Traynor > Change looks good to me. Acked-by: Eelco Chaudron ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 1/3] netdev-dpdk: Fix and document vhost tx retries.
On Fri, Jun 21, 2019 at 02:41:56PM +0100, Kevin Traynor wrote: > Fix minor issue of one additional retry and add > documentation about vhost tx retries and ways to > reduce/remove them. > > Signed-off-by: Kevin Traynor > > --- > There is a checkpatch warning that one of the libvirt > lines in docs is a few characters too long. It is similar > for some others in the file and I think it makes sense to > leave it as is but can wrap if required. > --- > Documentation/topics/dpdk/vhost-user.rst | 31 > lib/netdev-dpdk.c| 2 +- > 2 files changed, 32 insertions(+), 1 deletion(-) > > diff --git a/Documentation/topics/dpdk/vhost-user.rst > b/Documentation/topics/dpdk/vhost-user.rst > index f7b4b338e..d1682fd05 100644 > --- a/Documentation/topics/dpdk/vhost-user.rst > +++ b/Documentation/topics/dpdk/vhost-user.rst > @@ -76,4 +76,35 @@ mode ports require QEMU version 2.7. Ports of type > vhost-user are currently > deprecated and will be removed in a future release. > > +vhost tx retries > + > + > +When sending a batch of packets (max is 32) to a vhost-user or Maybe use the actual define name (NETDEV_MAX_BURST) since it might change? > +vhost-user-client interface, it may happen that some but not all of the > packets > +in the batch are able to be sent to the guest. This is often because there is > +not enough free descriptors in the virtqueue for all the packets to be sent. > +In this case there will be a retry, with a default maximum of 8 occurring. If > +at any time no packets can be sent, it may mean the guest is not accepting > +packets, so there are no (more) retries. > + > +Tx Retries may be reduced or removed by some external configuration, such s/removed/even avoided/ ? > +as increasing the virtqueue size through the ``rx_queue_size`` parameter in > +libvirt:: > + > + > + > + > + > + > + function='0x0'/> > + Do we need to worry about the qemu version? > + > +The guest application will also need need to provide enough descriptors. For > +example with ``testpmd`` the command line argument can be used:: > + > + --rxd=1024 --txd=1024 > + > +The guest should also have sufficient cores dedicated for consuming and > +processing packets at the required rate. > + > .. _dpdk-vhost-user: > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c > index 4856c56aa..0f3b9c6f4 100644 > --- a/lib/netdev-dpdk.c > +++ b/lib/netdev-dpdk.c > @@ -2353,5 +2353,5 @@ __netdev_dpdk_vhost_send(struct netdev *netdev, int qid, > break; > } > -} while (cnt && (retries++ <= VHOST_ENQ_RETRY_NUM)); > +} while (cnt && (retries++ < VHOST_ENQ_RETRY_NUM)); You're removing the packet's last hope to reach home, are you sure? :-) fbl > > rte_spinlock_unlock(>tx_q[qid].tx_lock); > -- > 2.20.1 > > ___ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 1/3] netdev-dpdk: Fix and document vhost tx retries.
Bleep bloop. Greetings Kevin Traynor, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: WARNING: Line is 81 characters long (recommended limit is 79) #44 FILE: Documentation/topics/dpdk/vhost-user.rst:98: Lines checked: 73, Warnings: 1, Errors: 0 Please check this out. If you feel there has been an error, please email acon...@bytheb.org Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 1/3] netdev-dpdk: Fix and document vhost tx retries.
Fix minor issue of one additional retry and add documentation about vhost tx retries and ways to reduce/remove them. Signed-off-by: Kevin Traynor --- There is a checkpatch warning that one of the libvirt lines in docs is a few characters too long. It is similar for some others in the file and I think it makes sense to leave it as is but can wrap if required. --- Documentation/topics/dpdk/vhost-user.rst | 31 lib/netdev-dpdk.c| 2 +- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/Documentation/topics/dpdk/vhost-user.rst b/Documentation/topics/dpdk/vhost-user.rst index f7b4b338e..d1682fd05 100644 --- a/Documentation/topics/dpdk/vhost-user.rst +++ b/Documentation/topics/dpdk/vhost-user.rst @@ -76,4 +76,35 @@ mode ports require QEMU version 2.7. Ports of type vhost-user are currently deprecated and will be removed in a future release. +vhost tx retries + + +When sending a batch of packets (max is 32) to a vhost-user or +vhost-user-client interface, it may happen that some but not all of the packets +in the batch are able to be sent to the guest. This is often because there is +not enough free descriptors in the virtqueue for all the packets to be sent. +In this case there will be a retry, with a default maximum of 8 occurring. If +at any time no packets can be sent, it may mean the guest is not accepting +packets, so there are no (more) retries. + +Tx Retries may be reduced or removed by some external configuration, such +as increasing the virtqueue size through the ``rx_queue_size`` parameter in +libvirt:: + + + + + + + + + +The guest application will also need need to provide enough descriptors. For +example with ``testpmd`` the command line argument can be used:: + + --rxd=1024 --txd=1024 + +The guest should also have sufficient cores dedicated for consuming and +processing packets at the required rate. + .. _dpdk-vhost-user: diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 4856c56aa..0f3b9c6f4 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -2353,5 +2353,5 @@ __netdev_dpdk_vhost_send(struct netdev *netdev, int qid, break; } -} while (cnt && (retries++ <= VHOST_ENQ_RETRY_NUM)); +} while (cnt && (retries++ < VHOST_ENQ_RETRY_NUM)); rte_spinlock_unlock(>tx_q[qid].tx_lock); -- 2.20.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
