[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [UNIKRAFT/LWIP PATCH v4 2/5] lwipopts.h: Disable Ethernet frame padding




On 11/6/20 2:19 PM, Simon Kuenzer wrote:
On 06.11.20 12:28, Sharan Santhanam wrote:
Hello Simon,

Please find the comment inline.

Thanks & Regards
Sharan

On 10/30/20 7:31 PM, Simon Kuenzer wrote:
From: Costin Lupu <costin.lupu@xxxxxxxxx>

We set ETH_PAD_SIZE to 0 considering that padding and alignment is
handled according to each driver capabilities.

Signed-off-by: Costin Lupu <costin.lupu@xxxxxxxxx>
Signed-off-by: Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
---
  include/arch/cc.h | 4 +++-
  uknetdev.c        | 6 ++++++
  2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/include/arch/cc.h b/include/arch/cc.h
index a1d0c34..b4526e0 100644
--- a/include/arch/cc.h
+++ b/include/arch/cc.h
@@ -51,7 +51,9 @@
  /* 32 bit checksum calculation */
  #define LWIP_CHKSUM_ALGORITHM 3
-#define ETH_PAD_SIZE 2
+
+/* Disable padding on Ethernet frames (only some uknetdev driver support it) */
+#define ETH_PAD_SIZE 0
If we make this driver dependent, shouldn't this be driven by either KConfig  hidden option.

Our idea was to set this as a global fixed thing which cannot be configured. According to https://lwn.net/Articles/89597/, the gain of doing padding seems to be negligible on the CPU architecture that we support but it may hurt DMA engines that may get involved on the hypervisor side. Additionally, it will cause issues with netfront because the netfront protocol does not support setting an offset for reception. Padding can then only be achieved by moving the packet bytes in memory which is obviously ways too costly. So, we think it should disabled it completely. I can add this as a comment to the commit message or as note to the code. What do you think?

Sure, we can add a comment in code.


Thanks,

Simon

  /* rand */
  #define LWIP_RAND() uk_swrand_randr()
diff --git a/uknetdev.c b/uknetdev.c
index e645ac1..ccb7368 100644
--- a/uknetdev.c
+++ b/uknetdev.c
@@ -259,6 +259,12 @@ static void uknetdev_input(struct uk_netdev *dev,
          /* Send packet to lwip */
  #if ETH_PAD_SIZE
+        /* If the following assertion fails, the driver most likely does
+         * not support padded receive. In such a case ETH_PAD_SIZE has
+         * to be set to 0.
+         */
+        UK_ASSERT(uk_netbuf_headroom(nb) >= ETH_PAD_SIZE);
+
          uk_netbuf_header(nb, ETH_PAD_SIZE);
  #endif /* ETH_PAD_SIZE */
          p = lwip_netbuf_to_pbuf(nb);



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.