Re: [PATCH 04/40] igb: Include the second VLAN tag in the buffer

2023-04-14 Thread Philippe Mathieu-Daudé 

On 14/4/23 16:32, Philippe Mathieu-Daudé wrote:

On 14/4/23 16:28, Philippe Mathieu-Daudé wrote:

On 14/4/23 13:37, Akihiko Odaki wrote:

Signed-off-by: Akihiko Odaki 
---
  hw/net/igb_core.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c
index 55de212447..f725ab97ae 100644
--- a/hw/net/igb_core.c
+++ b/hw/net/igb_core.c
@@ -1590,7 +1590,7 @@ static ssize_t
  igb_receive_internal(IGBCore *core, const struct iovec *iov, int 
iovcnt,

   bool has_vnet, bool *external_tx)
  {
-    static const int maximum_ethernet_hdr_len = (ETH_HLEN + 4);
+    static const int maximum_ethernet_hdr_len = (ETH_HLEN + 8);


Aren't VLAN tags 16-bit wide? Could you convert this magic value
to some verbose-but-obvious definitions?


Digging a bit more, is this struct vlan_header?


And now I see in patch #08 "igb: Always copy ethernet header":

  +typedef struct L2Header {
  +struct eth_header eth;
  +struct vlan_header vlan[2];
  +} L2Header;

Maybe add it first, and use sizeof(L2Header) here directly?

Re: [PATCH 04/40] igb: Include the second VLAN tag in the buffer

2023-04-14 Thread Philippe Mathieu-Daudé 

On 14/4/23 16:28, Philippe Mathieu-Daudé wrote:

On 14/4/23 13:37, Akihiko Odaki wrote:

Signed-off-by: Akihiko Odaki 
---
  hw/net/igb_core.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c
index 55de212447..f725ab97ae 100644
--- a/hw/net/igb_core.c
+++ b/hw/net/igb_core.c
@@ -1590,7 +1590,7 @@ static ssize_t
  igb_receive_internal(IGBCore *core, const struct iovec *iov, int 
iovcnt,

   bool has_vnet, bool *external_tx)
  {
-    static const int maximum_ethernet_hdr_len = (ETH_HLEN + 4);
+    static const int maximum_ethernet_hdr_len = (ETH_HLEN + 8);


Aren't VLAN tags 16-bit wide? Could you convert this magic value
to some verbose-but-obvious definitions?


Digging a bit more, is this struct vlan_header?


Is it worth adding a vlan_tag_t typedef in include/net/eth.h?


  uint16_t queues = 0;
  uint32_t n = 0;

Re: [PATCH 04/40] igb: Include the second VLAN tag in the buffer

2023-04-14 Thread Philippe Mathieu-Daudé 

On 14/4/23 13:37, Akihiko Odaki wrote:

Signed-off-by: Akihiko Odaki 
---
  hw/net/igb_core.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c
index 55de212447..f725ab97ae 100644
--- a/hw/net/igb_core.c
+++ b/hw/net/igb_core.c
@@ -1590,7 +1590,7 @@ static ssize_t
  igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
   bool has_vnet, bool *external_tx)
  {
-static const int maximum_ethernet_hdr_len = (ETH_HLEN + 4);
+static const int maximum_ethernet_hdr_len = (ETH_HLEN + 8);


Aren't VLAN tags 16-bit wide? Could you convert this magic value
to some verbose-but-obvious definitions?

Is it worth adding a vlan_tag_t typedef in include/net/eth.h?


  uint16_t queues = 0;
  uint32_t n = 0;

[PATCH 04/40] igb: Include the second VLAN tag in the buffer

2023-04-14 Thread Akihiko Odaki 
Signed-off-by: Akihiko Odaki 
---
 hw/net/igb_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c
index 55de212447..f725ab97ae 100644
--- a/hw/net/igb_core.c
+++ b/hw/net/igb_core.c
@@ -1590,7 +1590,7 @@ static ssize_t
 igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
  bool has_vnet, bool *external_tx)
 {
-static const int maximum_ethernet_hdr_len = (ETH_HLEN + 4);
+static const int maximum_ethernet_hdr_len = (ETH_HLEN + 8);
 
 uint16_t queues = 0;
 uint32_t n = 0;
-- 
2.40.0