[2/2] net/ark: remove RTE_LIBRTE_ARK_PAD_TX configuration macro
Checks
Commit Message
Replace behavior with RTE_LIBRTE_ARK_MIN_TX_PKTLEN
with a default value of 0.
Update documentation as needed.
Signed-off-by: Ed Czeck <ed.czeck@atomicrules.com>
---
doc/guides/nics/ark.rst | 13 ++++++++--
drivers/net/ark/ark_ethdev_tx.c | 44 +++++++++++++++++++--------------
drivers/net/ark/ark_logs.h | 8 ------
3 files changed, 36 insertions(+), 29 deletions(-)
Comments
On 8/24/2020 2:36 PM, Ed Czeck wrote:
> Replace behavior with RTE_LIBRTE_ARK_MIN_TX_PKTLEN
> with a default value of 0.
> Update documentation as needed.
>
> Signed-off-by: Ed Czeck <ed.czeck@atomicrules.com>
> ---
> doc/guides/nics/ark.rst | 13 ++++++++--
> drivers/net/ark/ark_ethdev_tx.c | 44 +++++++++++++++++++--------------
> drivers/net/ark/ark_logs.h | 8 ------
> 3 files changed, 36 insertions(+), 29 deletions(-)
>
> diff --git a/doc/guides/nics/ark.rst b/doc/guides/nics/ark.rst
> index c3ffcbbc2..f4d4539e5 100644
> --- a/doc/guides/nics/ark.rst
> +++ b/doc/guides/nics/ark.rst
> @@ -129,8 +129,10 @@ Configuration Information
> * **CONFIG_RTE_LIBRTE_ARK_PMD** (default y): Enables or disables inclusion
> of the ARK PMD driver in the DPDK compilation.
>
> - * **CONFIG_RTE_LIBRTE_ARK_PAD_TX** (default y): When enabled TX
> - packets are padded to 60 bytes to support downstream MACS.
> + * **CONFIG_RTE_LIBRTE_ARK_MIN_TX_PKTLEN** (default 0): Sets the minimum
> + packet length for tx packets to the FPGA. Packets less than this
> + length are padded to meet the requirement. This allows padding to
> + be offloaded or remain in host software.
>
There is no 'CONFIG_RTE_LIBRTE_ARK_MIN_TX_PKTLEN' option, I think it is
confusing to add it to documentation. There is already some documentation in
below for 'RTE_LIBRTE_ARK_MIN_TX_PKTLEN', what do you think removing this part?
And can you please remove 'CONFIG_RTE_LIBRTE_ARK_PAD_TX' form 'config/common_base'?
Another option is, do you think does it help your users to keep the config name
same, like keep the 'CONFIG_RTE_LIBRTE_ARK_PAD_TX' as 'RTE_LIBRTE_ARK_PAD_TX'
for the meson CFLAGs, instead of renaming it to 'RTE_LIBRTE_ARK_MIN_TX_PKTLEN'?
On Mon, Aug 24, 2020 at 10:55 AM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>
> On 8/24/2020 2:36 PM, Ed Czeck wrote:
> > Replace behavior with RTE_LIBRTE_ARK_MIN_TX_PKTLEN
> > with a default value of 0.
> > Update documentation as needed.
> >
> > Signed-off-by: Ed Czeck <ed.czeck@atomicrules.com>
> > ---
> > doc/guides/nics/ark.rst | 13 ++++++++--
> > drivers/net/ark/ark_ethdev_tx.c | 44 +++++++++++++++++++--------------
> > drivers/net/ark/ark_logs.h | 8 ------
> > 3 files changed, 36 insertions(+), 29 deletions(-)
> >
> > diff --git a/doc/guides/nics/ark.rst b/doc/guides/nics/ark.rst
> > index c3ffcbbc2..f4d4539e5 100644
> > --- a/doc/guides/nics/ark.rst
> > +++ b/doc/guides/nics/ark.rst
> > @@ -129,8 +129,10 @@ Configuration Information
> > * **CONFIG_RTE_LIBRTE_ARK_PMD** (default y): Enables or disables inclusion
> > of the ARK PMD driver in the DPDK compilation.
> >
> > - * **CONFIG_RTE_LIBRTE_ARK_PAD_TX** (default y): When enabled TX
> > - packets are padded to 60 bytes to support downstream MACS.
> > + * **CONFIG_RTE_LIBRTE_ARK_MIN_TX_PKTLEN** (default 0): Sets the minimum
> > + packet length for tx packets to the FPGA. Packets less than this
> > + length are padded to meet the requirement. This allows padding to
> > + be offloaded or remain in host software.
> >
>
> There is no 'CONFIG_RTE_LIBRTE_ARK_MIN_TX_PKTLEN' option, I think it is
> confusing to add it to documentation. There is already some documentation in
> below for 'RTE_LIBRTE_ARK_MIN_TX_PKTLEN', what do you think removing this part?
>
Should I remove the all references to the CONFIG_RTE_LIBRTE_ARK from
the net/ark section of the guide? It appears that CONFIG_.* will be
removed along with the makefile build system.
> And can you please remove 'CONFIG_RTE_LIBRTE_ARK_PAD_TX' form 'config/common_base'?
>
I am hesitant to modify config/common_base, since it is slated for
removal and edits will cause merge conflicts. Are you OK with this?
> Another option is, do you think does it help your users to keep the config name
> same, like keep the 'CONFIG_RTE_LIBRTE_ARK_PAD_TX' as 'RTE_LIBRTE_ARK_PAD_TX'
> for the meson CFLAGs, instead of renaming it to 'RTE_LIBRTE_ARK_MIN_TX_PKTLEN'?
I will drop the CONFIG names and leave the compiler marco documented
in my next patch.
Thx,
On 8/24/2020 10:51 PM, Ed Czeck wrote:
> On Mon, Aug 24, 2020 at 10:55 AM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>
>> On 8/24/2020 2:36 PM, Ed Czeck wrote:
>>> Replace behavior with RTE_LIBRTE_ARK_MIN_TX_PKTLEN
>>> with a default value of 0.
>>> Update documentation as needed.
>>>
>>> Signed-off-by: Ed Czeck <ed.czeck@atomicrules.com>
>>> ---
>>> doc/guides/nics/ark.rst | 13 ++++++++--
>>> drivers/net/ark/ark_ethdev_tx.c | 44 +++++++++++++++++++--------------
>>> drivers/net/ark/ark_logs.h | 8 ------
>>> 3 files changed, 36 insertions(+), 29 deletions(-)
>>>
>>> diff --git a/doc/guides/nics/ark.rst b/doc/guides/nics/ark.rst
>>> index c3ffcbbc2..f4d4539e5 100644
>>> --- a/doc/guides/nics/ark.rst
>>> +++ b/doc/guides/nics/ark.rst
>>> @@ -129,8 +129,10 @@ Configuration Information
>>> * **CONFIG_RTE_LIBRTE_ARK_PMD** (default y): Enables or disables inclusion
>>> of the ARK PMD driver in the DPDK compilation.
>>>
>>> - * **CONFIG_RTE_LIBRTE_ARK_PAD_TX** (default y): When enabled TX
>>> - packets are padded to 60 bytes to support downstream MACS.
>>> + * **CONFIG_RTE_LIBRTE_ARK_MIN_TX_PKTLEN** (default 0): Sets the minimum
>>> + packet length for tx packets to the FPGA. Packets less than this
>>> + length are padded to meet the requirement. This allows padding to
>>> + be offloaded or remain in host software.
>>>
>>
>> There is no 'CONFIG_RTE_LIBRTE_ARK_MIN_TX_PKTLEN' option, I think it is
>> confusing to add it to documentation. There is already some documentation in
>> below for 'RTE_LIBRTE_ARK_MIN_TX_PKTLEN', what do you think removing this part?
>>
> Should I remove the all references to the CONFIG_RTE_LIBRTE_ARK from
> the net/ark section of the guide? It appears that CONFIG_.* will be
> removed along with the makefile build system.
As you said it is removed by Ciara's patch, no need to touch it in this patch.
>
>> And can you please remove 'CONFIG_RTE_LIBRTE_ARK_PAD_TX' form 'config/common_base'?
>>
> I am hesitant to modify config/common_base, since it is slated for
> removal and edits will cause merge conflicts. Are you OK with this?
OK.
>
>> Another option is, do you think does it help your users to keep the config name
>> same, like keep the 'CONFIG_RTE_LIBRTE_ARK_PAD_TX' as 'RTE_LIBRTE_ARK_PAD_TX'
>> for the meson CFLAGs, instead of renaming it to 'RTE_LIBRTE_ARK_MIN_TX_PKTLEN'?
> I will drop the CONFIG names and leave the compiler marco documented
> in my next patch.
>
OK, thanks.
@@ -129,8 +129,10 @@ Configuration Information
* **CONFIG_RTE_LIBRTE_ARK_PMD** (default y): Enables or disables inclusion
of the ARK PMD driver in the DPDK compilation.
- * **CONFIG_RTE_LIBRTE_ARK_PAD_TX** (default y): When enabled TX
- packets are padded to 60 bytes to support downstream MACS.
+ * **CONFIG_RTE_LIBRTE_ARK_MIN_TX_PKTLEN** (default 0): Sets the minimum
+ packet length for tx packets to the FPGA. Packets less than this
+ length are padded to meet the requirement. This allows padding to
+ be offloaded or remain in host software.
Building DPDK
@@ -144,6 +146,13 @@ By default the ARK PMD library will be built into the DPDK library.
For configuring and using UIO and VFIO frameworks, please also refer :ref:`the
documentation that comes with DPDK suite <linux_gsg>`.
+To build with a non-zero minimum tx packet length, set the above macro in your
+CFLAGS environment prior to the meson build step. I.e.,
+
+ export CFLAGS="-DRTE_LIBRTE_ARK_MIN_TX_PKTLEN=60"
+ meson build
+
+
Supported ARK RTL PCIe Instances
--------------------------------
@@ -15,6 +15,13 @@
#define ARK_TX_MAX_NOCHAIN (RTE_MBUF_DEFAULT_DATAROOM)
+#ifndef RTE_LIBRTE_ARK_MIN_TX_PKTLEN
+#define ARK_MIN_TX_PKTLEN 0
+#else
+#define ARK_MIN_TX_PKTLEN RTE_LIBRTE_ARK_MIN_TX_PKTLEN
+#endif
+
+
/* ************************************************************************* */
struct ark_tx_queue {
struct ark_tx_meta *meta_q;
@@ -91,6 +98,7 @@ eth_ark_xmit_pkts(void *vtxq, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
uint32_t prod_index_limit;
int stat;
uint16_t nb;
+ const uint32_t min_pkt_len = ARK_MIN_TX_PKTLEN;
queue = (struct ark_tx_queue *)vtxq;
@@ -104,27 +112,25 @@ eth_ark_xmit_pkts(void *vtxq, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
++nb) {
mbuf = tx_pkts[nb];
- if (ARK_TX_PAD_TO_60) {
- if (unlikely(rte_pktmbuf_pkt_len(mbuf) < 60)) {
- /* this packet even if it is small can be split,
- * be sure to add to the end mbuf
+ if (unlikely(rte_pktmbuf_pkt_len(mbuf) < min_pkt_len)) {
+ /* this packet even if it is small can be split,
+ * be sure to add to the end mbuf
+ */
+ uint16_t to_add = min_pkt_len -
+ rte_pktmbuf_pkt_len(mbuf);
+ char *appended =
+ rte_pktmbuf_append(mbuf, to_add);
+
+ if (appended == 0) {
+ /* This packet is in error,
+ * we cannot send it so just
+ * count it and delete it.
*/
- uint16_t to_add =
- 60 - rte_pktmbuf_pkt_len(mbuf);
- char *appended =
- rte_pktmbuf_append(mbuf, to_add);
-
- if (appended == 0) {
- /* This packet is in error,
- * we cannot send it so just
- * count it and delete it.
- */
- queue->tx_errors += 1;
- rte_pktmbuf_free(mbuf);
- continue;
- }
- memset(appended, 0, to_add);
+ queue->tx_errors += 1;
+ rte_pktmbuf_free(mbuf);
+ continue;
}
+ memset(appended, 0, to_add);
}
if (unlikely(mbuf->nb_segs != 1)) {
@@ -8,14 +8,6 @@
#include <inttypes.h>
#include <rte_log.h>
-
-/* Configuration option to pad TX packets to 60 bytes */
-#ifdef RTE_LIBRTE_ARK_PAD_TX
-#define ARK_TX_PAD_TO_60 1
-#else
-#define ARK_TX_PAD_TO_60 0
-#endif
-
/* system camel case definition changed to upper case */
#define PRIU32 PRIu32
#define PRIU64 PRIu64