* [PATCH] Updating the GEM MAC IP to properly filter out the multicast addresses
@ 2019-11-28 15:10 Wasim, Bilal
2019-11-28 15:59 ` Edgar E. Iglesias
0 siblings, 1 reply; 7+ messages in thread
From: Wasim, Bilal @ 2019-11-28 15:10 UTC (permalink / raw)
To: qemu-devel; +Cc: edgar.iglesias, alistair, qemu-arm, peter.maydell
[-- Attachment #1: Type: text/plain, Size: 2696 bytes --]
[PATCH] Updating the GEM MAC IP to properly filter out the multicast
addresses. The current code makes a bad assumption that the most-significant
byte of the MAC address is used to determine if the address is multicast or
unicast, but in reality only a single bit is used to determine this. This
caused IPv6 to not work.. Fix is now in place and has been tested with
ZCU102-A53 / IPv6 on a TAP interface. Works well..
Signed-off-by: Bilal Wasim <bilal_wasim@mentor.com>
---
hw/net/cadence_gem.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index b8be73d..f8bcbb3 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -315,6 +315,12 @@
#define GEM_MODID_VALUE 0x00020118
+/* IEEE has specified that the most significant bit of the most significant byte be used for
+ * distinguishing between Unicast and Multicast addresses.
+ * If its a 1, that means multicast, 0 means unicast. */
+#define IS_MULTICAST(address) (((address[0] & 0x01) == 0x01) ? 1 : 0)
+#define IS_UNICAST(address) (!IS_MULTICAST(address))
+
static inline uint64_t tx_desc_get_buffer(CadenceGEMState *s, uint32_t *desc)
{
uint64_t ret = desc[0];
@@ -690,21 +696,21 @@ static int gem_mac_address_filter(CadenceGEMState *s, const uint8_t *packet)
}
/* Accept packets -w- hash match? */
- if ((packet[0] == 0x01 && (s->regs[GEM_NWCFG] & GEM_NWCFG_MCAST_HASH)) ||
- (packet[0] != 0x01 && (s->regs[GEM_NWCFG] & GEM_NWCFG_UCAST_HASH))) {
+ if ((IS_MULTICAST(packet) && (s->regs[GEM_NWCFG] & GEM_NWCFG_MCAST_HASH)) ||
+ (IS_UNICAST(packet) && (s->regs[GEM_NWCFG] & GEM_NWCFG_UCAST_HASH))) {
unsigned hash_index;
hash_index = calc_mac_hash(packet);
if (hash_index < 32) {
if (s->regs[GEM_HASHLO] & (1<<hash_index)) {
- return packet[0] == 0x01 ? GEM_RX_MULTICAST_HASH_ACCEPT :
- GEM_RX_UNICAST_HASH_ACCEPT;
+ return IS_MULTICAST(packet) ? GEM_RX_MULTICAST_HASH_ACCEPT :
+ GEM_RX_UNICAST_HASH_ACCEPT;
}
} else {
hash_index -= 32;
if (s->regs[GEM_HASHHI] & (1<<hash_index)) {
- return packet[0] == 0x01 ? GEM_RX_MULTICAST_HASH_ACCEPT :
- GEM_RX_UNICAST_HASH_ACCEPT;
+ return IS_MULTICAST(packet) ? GEM_RX_MULTICAST_HASH_ACCEPT :
+ GEM_RX_UNICAST_HASH_ACCEPT;
}
}
}
--
2.9.3
[-- Attachment #2: Type: text/html, Size: 8711 bytes --]
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] Updating the GEM MAC IP to properly filter out the multicast addresses
2019-11-28 15:10 [PATCH] Updating the GEM MAC IP to properly filter out the multicast addresses Wasim, Bilal
@ 2019-11-28 15:59 ` Edgar E. Iglesias
2019-11-28 17:02 ` Wasim, Bilal
0 siblings, 1 reply; 7+ messages in thread
From: Edgar E. Iglesias @ 2019-11-28 15:59 UTC (permalink / raw)
To: Wasim, Bilal; +Cc: peter.maydell, alistair, qemu-devel, qemu-arm
On Thu, Nov 28, 2019 at 03:10:16PM +0000, Wasim, Bilal wrote:
> [PATCH] Updating the GEM MAC IP to properly filter out the multicast
> addresses. The current code makes a bad assumption that the most-significant
> byte of the MAC address is used to determine if the address is multicast or
> unicast, but in reality only a single bit is used to determine this. This
> caused IPv6 to not work.. Fix is now in place and has been tested with
> ZCU102-A53 / IPv6 on a TAP interface. Works well..
Hi Bilal,
The fix looks right to me but I have a few comments.
* Your patch seems a little wrongly formated.
[PATCH] goes into the Subject line for example and you're missing
path prefixes.
Do a git log -- hw/net/cadence_gem.c to see examples on how it should look.
* The patch will probably not pass checkpatch since you seem to have
long lines.
* We also need to update gem_receive_updatestats() to use the corrected macros.
More inline:
>
> Signed-off-by: Bilal Wasim <bilal_wasim@mentor.com>
> ---
> hw/net/cadence_gem.c | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
> index b8be73d..f8bcbb3 100644
> --- a/hw/net/cadence_gem.c
> +++ b/hw/net/cadence_gem.c
> @@ -315,6 +315,12 @@
> #define GEM_MODID_VALUE 0x00020118
> +/* IEEE has specified that the most significant bit of the most significant byte be used for
> + * distinguishing between Unicast and Multicast addresses.
> + * If its a 1, that means multicast, 0 means unicast. */
> +#define IS_MULTICAST(address) (((address[0] & 0x01) == 0x01) ? 1 : 0)
This can be simplified:
#define IS_MULTICAST(address) (address[0] & 1)
Actually, looking closer, we already have functions to do these checks in:
include/net/eth.h
static inline int is_multicast_ether_addr(const uint8_t *addr)
static inline int is_broadcast_ether_addr(const uint8_t *addr)
static inline int is_unicast_ether_addr(const uint8_t *addr)
> +#define IS_UNICAST(address) (!IS_MULTICAST(address))
> +
> static inline uint64_t tx_desc_get_buffer(CadenceGEMState *s, uint32_t *desc)
> {
> uint64_t ret = desc[0];
> @@ -690,21 +696,21 @@ static int gem_mac_address_filter(CadenceGEMState *s, const uint8_t *packet)
> }
> /* Accept packets -w- hash match? */
> - if ((packet[0] == 0x01 && (s->regs[GEM_NWCFG] & GEM_NWCFG_MCAST_HASH)) ||
> - (packet[0] != 0x01 && (s->regs[GEM_NWCFG] & GEM_NWCFG_UCAST_HASH))) {
> + if ((IS_MULTICAST(packet) && (s->regs[GEM_NWCFG] & GEM_NWCFG_MCAST_HASH)) ||
> + (IS_UNICAST(packet) && (s->regs[GEM_NWCFG] & GEM_NWCFG_UCAST_HASH))) {
> unsigned hash_index;
> hash_index = calc_mac_hash(packet);
> if (hash_index < 32) {
> if (s->regs[GEM_HASHLO] & (1<<hash_index)) {
> - return packet[0] == 0x01 ? GEM_RX_MULTICAST_HASH_ACCEPT :
> - GEM_RX_UNICAST_HASH_ACCEPT;
> + return IS_MULTICAST(packet) ? GEM_RX_MULTICAST_HASH_ACCEPT :
> + GEM_RX_UNICAST_HASH_ACCEPT;
> }
> } else {
> hash_index -= 32;
> if (s->regs[GEM_HASHHI] & (1<<hash_index)) {
> - return packet[0] == 0x01 ? GEM_RX_MULTICAST_HASH_ACCEPT :
> - GEM_RX_UNICAST_HASH_ACCEPT;
> + return IS_MULTICAST(packet) ? GEM_RX_MULTICAST_HASH_ACCEPT :
> + GEM_RX_UNICAST_HASH_ACCEPT;
> }
> }
> }
> --
> 2.9.3
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH] Updating the GEM MAC IP to properly filter out the multicast addresses
2019-11-28 15:59 ` Edgar E. Iglesias
@ 2019-11-28 17:02 ` Wasim, Bilal
2019-11-28 17:31 ` Edgar E. Iglesias
2019-11-29 10:00 ` no-reply
0 siblings, 2 replies; 7+ messages in thread
From: Wasim, Bilal @ 2019-11-28 17:02 UTC (permalink / raw)
To: Edgar E. Iglesias; +Cc: peter.maydell, alistair, qemu-devel, qemu-arm
This was one of my first attempts, and so I was sure to miss something.. I've incorporated all the updates in this patch.. Let me know what you think about this..
net/cadence_gem: Updating the GEM MAC IP to properly filter out the multicast addresses.
The current code makes a bad assumption that the most-significant byte
of the MAC address is used to determine if the address is multicast or
unicast, but in reality only a single bit is used to determine this.
This caused IPv6 to not work.. Fix is now in place and has been tested
with ZCU102-A53 / IPv6 on a TAP interface. Works well..
Signed-off-by: Bilal Wasim <bilal_wasim@mentor.com>
---
hw/net/cadence_gem.c | 21 ++++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index b8be73dc55..98efb93f8a 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -34,6 +34,7 @@
#include "qemu/module.h"
#include "sysemu/dma.h"
#include "net/checksum.h"
+#include "net/eth.h"
#ifdef CADENCE_GEM_ERR_DEBUG
#define DB_PRINT(...) do { \
@@ -315,6 +316,12 @@
#define GEM_MODID_VALUE 0x00020118
+/* IEEE has specified that the most significant bit of the most significant byte be used for
+ * distinguishing between Unicast and Multicast addresses.
+ * If its a 1, that means multicast, 0 means unicast. */
+#define IS_MULTICAST(address) is_multicast_ether_addr(address)
+#define IS_UNICAST(address) is_unicast_ether_addr(address)
+
static inline uint64_t tx_desc_get_buffer(CadenceGEMState *s, uint32_t *desc)
{
uint64_t ret = desc[0];
@@ -601,7 +608,7 @@ static void gem_receive_updatestats(CadenceGEMState *s, const uint8_t *packet,
}
/* Error-free Multicast Frames counter */
- if (packet[0] == 0x01) {
+ if (IS_MULTICAST(packet)) {
s->regs[GEM_RXMULTICNT]++;
}
@@ -690,21 +697,21 @@ static int gem_mac_address_filter(CadenceGEMState *s, const uint8_t *packet)
}
/* Accept packets -w- hash match? */
- if ((packet[0] == 0x01 && (s->regs[GEM_NWCFG] & GEM_NWCFG_MCAST_HASH)) ||
- (packet[0] != 0x01 && (s->regs[GEM_NWCFG] & GEM_NWCFG_UCAST_HASH))) {
+ if ((IS_MULTICAST(packet) && (s->regs[GEM_NWCFG] & GEM_NWCFG_MCAST_HASH)) ||
+ (IS_UNICAST(packet) && (s->regs[GEM_NWCFG] & GEM_NWCFG_UCAST_HASH))) {
unsigned hash_index;
hash_index = calc_mac_hash(packet);
if (hash_index < 32) {
if (s->regs[GEM_HASHLO] & (1<<hash_index)) {
- return packet[0] == 0x01 ? GEM_RX_MULTICAST_HASH_ACCEPT :
- GEM_RX_UNICAST_HASH_ACCEPT;
+ return IS_MULTICAST(packet) ? GEM_RX_MULTICAST_HASH_ACCEPT :
+ GEM_RX_UNICAST_HASH_ACCEPT;
}
} else {
hash_index -= 32;
if (s->regs[GEM_HASHHI] & (1<<hash_index)) {
- return packet[0] == 0x01 ? GEM_RX_MULTICAST_HASH_ACCEPT :
- GEM_RX_UNICAST_HASH_ACCEPT;
+ return IS_MULTICAST(packet) ? GEM_RX_MULTICAST_HASH_ACCEPT :
+ GEM_RX_UNICAST_HASH_ACCEPT;
}
}
}
--
2.19.1.windows.1
------------------------------------------------------------------------------------------------------------------------------
-----Original Message-----
From: Edgar E. Iglesias [mailto:edgar.iglesias@gmail.com]
Sent: Thursday, November 28, 2019 9:00 PM
To: Wasim, Bilal <Bilal_Wasim@mentor.com>
Cc: qemu-devel@nongnu.org; alistair@alistair23.me; peter.maydell@linaro.org; qemu-arm@nongnu.org
Subject: Re: [PATCH] Updating the GEM MAC IP to properly filter out the multicast addresses
On Thu, Nov 28, 2019 at 03:10:16PM +0000, Wasim, Bilal wrote:
> [PATCH] Updating the GEM MAC IP to properly filter out the multicast
> addresses. The current code makes a bad assumption that the
> most-significant byte of the MAC address is used to determine if the
> address is multicast or unicast, but in reality only a single bit is
> used to determine this. This caused IPv6 to not work.. Fix is now in
> place and has been tested with
> ZCU102-A53 / IPv6 on a TAP interface. Works well..
Hi Bilal,
The fix looks right to me but I have a few comments.
* Your patch seems a little wrongly formated.
[PATCH] goes into the Subject line for example and you're missing path prefixes.
Do a git log -- hw/net/cadence_gem.c to see examples on how it should look.
* The patch will probably not pass checkpatch since you seem to have long lines.
* We also need to update gem_receive_updatestats() to use the corrected macros.
More inline:
>
> Signed-off-by: Bilal Wasim <bilal_wasim@mentor.com>
> ---
> hw/net/cadence_gem.c | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c index
> b8be73d..f8bcbb3 100644
> --- a/hw/net/cadence_gem.c
> +++ b/hw/net/cadence_gem.c
> @@ -315,6 +315,12 @@
> #define GEM_MODID_VALUE 0x00020118
> +/* IEEE has specified that the most significant bit of the most
> +significant byte be used for
> + * distinguishing between Unicast and Multicast addresses.
> + * If its a 1, that means multicast, 0 means unicast. */
> +#define IS_MULTICAST(address) (((address[0] & 0x01) == 0x01) ? 1 : 0)
This can be simplified:
#define IS_MULTICAST(address) (address[0] & 1)
Actually, looking closer, we already have functions to do these checks in:
include/net/eth.h
static inline int is_multicast_ether_addr(const uint8_t *addr) static inline int is_broadcast_ether_addr(const uint8_t *addr) static inline int is_unicast_ether_addr(const uint8_t *addr)
> +#define IS_UNICAST(address) (!IS_MULTICAST(address))
> +
> static inline uint64_t tx_desc_get_buffer(CadenceGEMState *s, uint32_t
> *desc) {
> uint64_t ret = desc[0];
> @@ -690,21 +696,21 @@ static int gem_mac_address_filter(CadenceGEMState *s, const uint8_t *packet)
> }
> /* Accept packets -w- hash match? */
> - if ((packet[0] == 0x01 && (s->regs[GEM_NWCFG] & GEM_NWCFG_MCAST_HASH)) ||
> - (packet[0] != 0x01 && (s->regs[GEM_NWCFG] & GEM_NWCFG_UCAST_HASH))) {
> + if ((IS_MULTICAST(packet) && (s->regs[GEM_NWCFG] & GEM_NWCFG_MCAST_HASH)) ||
> + (IS_UNICAST(packet) && (s->regs[GEM_NWCFG] & GEM_NWCFG_UCAST_HASH))) {
> unsigned hash_index;
> hash_index = calc_mac_hash(packet);
> if (hash_index < 32) {
> if (s->regs[GEM_HASHLO] & (1<<hash_index)) {
> - return packet[0] == 0x01 ? GEM_RX_MULTICAST_HASH_ACCEPT :
> - GEM_RX_UNICAST_HASH_ACCEPT;
> + return IS_MULTICAST(packet) ? GEM_RX_MULTICAST_HASH_ACCEPT :
> +
> + GEM_RX_UNICAST_HASH_ACCEPT;
> }
> } else {
> hash_index -= 32;
> if (s->regs[GEM_HASHHI] & (1<<hash_index)) {
> - return packet[0] == 0x01 ? GEM_RX_MULTICAST_HASH_ACCEPT :
> - GEM_RX_UNICAST_HASH_ACCEPT;
> + return IS_MULTICAST(packet) ? GEM_RX_MULTICAST_HASH_ACCEPT :
> +
> + GEM_RX_UNICAST_HASH_ACCEPT;
> }
> }
> }
> --
> 2.9.3
>
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] Updating the GEM MAC IP to properly filter out the multicast addresses
2019-11-28 17:02 ` Wasim, Bilal
@ 2019-11-28 17:31 ` Edgar E. Iglesias
2019-11-28 17:34 ` Wasim, Bilal
2019-11-29 10:00 ` no-reply
1 sibling, 1 reply; 7+ messages in thread
From: Edgar E. Iglesias @ 2019-11-28 17:31 UTC (permalink / raw)
To: Wasim, Bilal; +Cc: peter.maydell, alistair, qemu-devel, qemu-arm
On Thu, Nov 28, 2019 at 05:02:00PM +0000, Wasim, Bilal wrote:
> This was one of my first attempts, and so I was sure to miss something.. I've incorporated all the updates in this patch.. Let me know what you think about this..
>
> net/cadence_gem: Updating the GEM MAC IP to properly filter out the multicast addresses.
>
> The current code makes a bad assumption that the most-significant byte
> of the MAC address is used to determine if the address is multicast or
> unicast, but in reality only a single bit is used to determine this.
> This caused IPv6 to not work.. Fix is now in place and has been tested
> with ZCU102-A53 / IPv6 on a TAP interface. Works well..
Thanks Bilal,
This looks better but not quite there yet.
* You don't seem to be using git-send-email to post patches, try it,
it will make life easier wrt to formatting. The patch should be in a
separate email. The subject line should be the subject of the email.
git-format-patch and git-send-email will take care of that for you.
* You don't need to define IS_MULTICAST, you can directly
use is_multicast_ether_addr() and friends.
* The patch still has long lines (longer than 80 chars)
You can run scripts/checkpatch.pl on your commit before
posting the patch.
Cheers,
Edgar
>
> Signed-off-by: Bilal Wasim <bilal_wasim@mentor.com>
> ---
> hw/net/cadence_gem.c | 21 ++++++++++++++-------
> 1 file changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
> index b8be73dc55..98efb93f8a 100644
> --- a/hw/net/cadence_gem.c
> +++ b/hw/net/cadence_gem.c
> @@ -34,6 +34,7 @@
> #include "qemu/module.h"
> #include "sysemu/dma.h"
> #include "net/checksum.h"
> +#include "net/eth.h"
>
> #ifdef CADENCE_GEM_ERR_DEBUG
> #define DB_PRINT(...) do { \
> @@ -315,6 +316,12 @@
>
> #define GEM_MODID_VALUE 0x00020118
>
> +/* IEEE has specified that the most significant bit of the most significant byte be used for
> + * distinguishing between Unicast and Multicast addresses.
> + * If its a 1, that means multicast, 0 means unicast. */
> +#define IS_MULTICAST(address) is_multicast_ether_addr(address)
> +#define IS_UNICAST(address) is_unicast_ether_addr(address)
> +
> static inline uint64_t tx_desc_get_buffer(CadenceGEMState *s, uint32_t *desc)
> {
> uint64_t ret = desc[0];
> @@ -601,7 +608,7 @@ static void gem_receive_updatestats(CadenceGEMState *s, const uint8_t *packet,
> }
>
> /* Error-free Multicast Frames counter */
> - if (packet[0] == 0x01) {
> + if (IS_MULTICAST(packet)) {
> s->regs[GEM_RXMULTICNT]++;
> }
>
> @@ -690,21 +697,21 @@ static int gem_mac_address_filter(CadenceGEMState *s, const uint8_t *packet)
> }
>
> /* Accept packets -w- hash match? */
> - if ((packet[0] == 0x01 && (s->regs[GEM_NWCFG] & GEM_NWCFG_MCAST_HASH)) ||
> - (packet[0] != 0x01 && (s->regs[GEM_NWCFG] & GEM_NWCFG_UCAST_HASH))) {
> + if ((IS_MULTICAST(packet) && (s->regs[GEM_NWCFG] & GEM_NWCFG_MCAST_HASH)) ||
> + (IS_UNICAST(packet) && (s->regs[GEM_NWCFG] & GEM_NWCFG_UCAST_HASH))) {
> unsigned hash_index;
>
> hash_index = calc_mac_hash(packet);
> if (hash_index < 32) {
> if (s->regs[GEM_HASHLO] & (1<<hash_index)) {
> - return packet[0] == 0x01 ? GEM_RX_MULTICAST_HASH_ACCEPT :
> - GEM_RX_UNICAST_HASH_ACCEPT;
> + return IS_MULTICAST(packet) ? GEM_RX_MULTICAST_HASH_ACCEPT :
> + GEM_RX_UNICAST_HASH_ACCEPT;
> }
> } else {
> hash_index -= 32;
> if (s->regs[GEM_HASHHI] & (1<<hash_index)) {
> - return packet[0] == 0x01 ? GEM_RX_MULTICAST_HASH_ACCEPT :
> - GEM_RX_UNICAST_HASH_ACCEPT;
> + return IS_MULTICAST(packet) ? GEM_RX_MULTICAST_HASH_ACCEPT :
> + GEM_RX_UNICAST_HASH_ACCEPT;
> }
> }
> }
> --
> 2.19.1.windows.1
>
> ------------------------------------------------------------------------------------------------------------------------------
> -----Original Message-----
> From: Edgar E. Iglesias [mailto:edgar.iglesias@gmail.com]
> Sent: Thursday, November 28, 2019 9:00 PM
> To: Wasim, Bilal <Bilal_Wasim@mentor.com>
> Cc: qemu-devel@nongnu.org; alistair@alistair23.me; peter.maydell@linaro.org; qemu-arm@nongnu.org
> Subject: Re: [PATCH] Updating the GEM MAC IP to properly filter out the multicast addresses
>
> On Thu, Nov 28, 2019 at 03:10:16PM +0000, Wasim, Bilal wrote:
> > [PATCH] Updating the GEM MAC IP to properly filter out the multicast
> > addresses. The current code makes a bad assumption that the
> > most-significant byte of the MAC address is used to determine if the
> > address is multicast or unicast, but in reality only a single bit is
> > used to determine this. This caused IPv6 to not work.. Fix is now in
> > place and has been tested with
> > ZCU102-A53 / IPv6 on a TAP interface. Works well..
>
> Hi Bilal,
>
> The fix looks right to me but I have a few comments.
>
> * Your patch seems a little wrongly formated.
> [PATCH] goes into the Subject line for example and you're missing path prefixes.
>
> Do a git log -- hw/net/cadence_gem.c to see examples on how it should look.
>
> * The patch will probably not pass checkpatch since you seem to have long lines.
>
> * We also need to update gem_receive_updatestats() to use the corrected macros.
>
> More inline:
>
> >
> > Signed-off-by: Bilal Wasim <bilal_wasim@mentor.com>
> > ---
> > hw/net/cadence_gem.c | 18 ++++++++++++------
> > 1 file changed, 12 insertions(+), 6 deletions(-)
> >
> > diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c index
> > b8be73d..f8bcbb3 100644
> > --- a/hw/net/cadence_gem.c
> > +++ b/hw/net/cadence_gem.c
> > @@ -315,6 +315,12 @@
> > #define GEM_MODID_VALUE 0x00020118
> > +/* IEEE has specified that the most significant bit of the most
> > +significant byte be used for
> > + * distinguishing between Unicast and Multicast addresses.
> > + * If its a 1, that means multicast, 0 means unicast. */
> > +#define IS_MULTICAST(address) (((address[0] & 0x01) == 0x01) ? 1 : 0)
>
>
>
> This can be simplified:
> #define IS_MULTICAST(address) (address[0] & 1)
>
> Actually, looking closer, we already have functions to do these checks in:
> include/net/eth.h
>
> static inline int is_multicast_ether_addr(const uint8_t *addr) static inline int is_broadcast_ether_addr(const uint8_t *addr) static inline int is_unicast_ether_addr(const uint8_t *addr)
>
>
>
> > +#define IS_UNICAST(address) (!IS_MULTICAST(address))
> > +
> > static inline uint64_t tx_desc_get_buffer(CadenceGEMState *s, uint32_t
> > *desc) {
> > uint64_t ret = desc[0];
> > @@ -690,21 +696,21 @@ static int gem_mac_address_filter(CadenceGEMState *s, const uint8_t *packet)
> > }
> > /* Accept packets -w- hash match? */
> > - if ((packet[0] == 0x01 && (s->regs[GEM_NWCFG] & GEM_NWCFG_MCAST_HASH)) ||
> > - (packet[0] != 0x01 && (s->regs[GEM_NWCFG] & GEM_NWCFG_UCAST_HASH))) {
> > + if ((IS_MULTICAST(packet) && (s->regs[GEM_NWCFG] & GEM_NWCFG_MCAST_HASH)) ||
> > + (IS_UNICAST(packet) && (s->regs[GEM_NWCFG] & GEM_NWCFG_UCAST_HASH))) {
> > unsigned hash_index;
> > hash_index = calc_mac_hash(packet);
> > if (hash_index < 32) {
> > if (s->regs[GEM_HASHLO] & (1<<hash_index)) {
> > - return packet[0] == 0x01 ? GEM_RX_MULTICAST_HASH_ACCEPT :
> > - GEM_RX_UNICAST_HASH_ACCEPT;
> > + return IS_MULTICAST(packet) ? GEM_RX_MULTICAST_HASH_ACCEPT :
> > +
> > + GEM_RX_UNICAST_HASH_ACCEPT;
> > }
> > } else {
> > hash_index -= 32;
> > if (s->regs[GEM_HASHHI] & (1<<hash_index)) {
> > - return packet[0] == 0x01 ? GEM_RX_MULTICAST_HASH_ACCEPT :
> > - GEM_RX_UNICAST_HASH_ACCEPT;
> > + return IS_MULTICAST(packet) ? GEM_RX_MULTICAST_HASH_ACCEPT :
> > +
> > + GEM_RX_UNICAST_HASH_ACCEPT;
> > }
> > }
> > }
> > --
> > 2.9.3
> >
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH] Updating the GEM MAC IP to properly filter out the multicast addresses
2019-11-28 17:31 ` Edgar E. Iglesias
@ 2019-11-28 17:34 ` Wasim, Bilal
2019-11-29 8:54 ` Wasim, Bilal
0 siblings, 1 reply; 7+ messages in thread
From: Wasim, Bilal @ 2019-11-28 17:34 UTC (permalink / raw)
To: Edgar E. Iglesias; +Cc: peter.maydell, alistair, qemu-devel, qemu-arm
Thanks for the pointers.. I will incorporate all these changes and post an updated thread asap..
-----Original Message-----
From: Edgar E. Iglesias [mailto:edgar.iglesias@gmail.com]
Sent: Thursday, November 28, 2019 10:32 PM
To: Wasim, Bilal <Bilal_Wasim@mentor.com>
Cc: qemu-devel@nongnu.org; alistair@alistair23.me; peter.maydell@linaro.org; qemu-arm@nongnu.org
Subject: Re: [PATCH] Updating the GEM MAC IP to properly filter out the multicast addresses
On Thu, Nov 28, 2019 at 05:02:00PM +0000, Wasim, Bilal wrote:
> This was one of my first attempts, and so I was sure to miss something.. I've incorporated all the updates in this patch.. Let me know what you think about this..
>
> net/cadence_gem: Updating the GEM MAC IP to properly filter out the multicast addresses.
>
> The current code makes a bad assumption that the most-significant byte
> of the MAC address is used to determine if the address is multicast or
> unicast, but in reality only a single bit is used to determine this.
> This caused IPv6 to not work.. Fix is now in place and has been tested
> with ZCU102-A53 / IPv6 on a TAP interface. Works well..
Thanks Bilal,
This looks better but not quite there yet.
* You don't seem to be using git-send-email to post patches, try it, it will make life easier wrt to formatting. The patch should be in a separate email. The subject line should be the subject of the email.
git-format-patch and git-send-email will take care of that for you.
* You don't need to define IS_MULTICAST, you can directly use is_multicast_ether_addr() and friends.
* The patch still has long lines (longer than 80 chars) You can run scripts/checkpatch.pl on your commit before posting the patch.
Cheers,
Edgar
>
> Signed-off-by: Bilal Wasim <bilal_wasim@mentor.com>
> ---
> hw/net/cadence_gem.c | 21 ++++++++++++++-------
> 1 file changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c index
> b8be73dc55..98efb93f8a 100644
> --- a/hw/net/cadence_gem.c
> +++ b/hw/net/cadence_gem.c
> @@ -34,6 +34,7 @@
> #include "qemu/module.h"
> #include "sysemu/dma.h"
> #include "net/checksum.h"
> +#include "net/eth.h"
>
> #ifdef CADENCE_GEM_ERR_DEBUG
> #define DB_PRINT(...) do { \
> @@ -315,6 +316,12 @@
>
> #define GEM_MODID_VALUE 0x00020118
>
> +/* IEEE has specified that the most significant bit of the most
> +significant byte be used for
> + * distinguishing between Unicast and Multicast addresses.
> + * If its a 1, that means multicast, 0 means unicast. */
> +#define IS_MULTICAST(address) is_multicast_ether_addr(address)
> +#define IS_UNICAST(address) is_unicast_ether_addr(address)
> +
> static inline uint64_t tx_desc_get_buffer(CadenceGEMState *s,
> uint32_t *desc) {
> uint64_t ret = desc[0];
> @@ -601,7 +608,7 @@ static void gem_receive_updatestats(CadenceGEMState *s, const uint8_t *packet,
> }
>
> /* Error-free Multicast Frames counter */
> - if (packet[0] == 0x01) {
> + if (IS_MULTICAST(packet)) {
> s->regs[GEM_RXMULTICNT]++;
> }
>
> @@ -690,21 +697,21 @@ static int gem_mac_address_filter(CadenceGEMState *s, const uint8_t *packet)
> }
>
> /* Accept packets -w- hash match? */
> - if ((packet[0] == 0x01 && (s->regs[GEM_NWCFG] & GEM_NWCFG_MCAST_HASH)) ||
> - (packet[0] != 0x01 && (s->regs[GEM_NWCFG] & GEM_NWCFG_UCAST_HASH))) {
> + if ((IS_MULTICAST(packet) && (s->regs[GEM_NWCFG] & GEM_NWCFG_MCAST_HASH)) ||
> + (IS_UNICAST(packet) && (s->regs[GEM_NWCFG] & GEM_NWCFG_UCAST_HASH))) {
> unsigned hash_index;
>
> hash_index = calc_mac_hash(packet);
> if (hash_index < 32) {
> if (s->regs[GEM_HASHLO] & (1<<hash_index)) {
> - return packet[0] == 0x01 ? GEM_RX_MULTICAST_HASH_ACCEPT :
> - GEM_RX_UNICAST_HASH_ACCEPT;
> + return IS_MULTICAST(packet) ? GEM_RX_MULTICAST_HASH_ACCEPT :
> +
> + GEM_RX_UNICAST_HASH_ACCEPT;
> }
> } else {
> hash_index -= 32;
> if (s->regs[GEM_HASHHI] & (1<<hash_index)) {
> - return packet[0] == 0x01 ? GEM_RX_MULTICAST_HASH_ACCEPT :
> - GEM_RX_UNICAST_HASH_ACCEPT;
> + return IS_MULTICAST(packet) ? GEM_RX_MULTICAST_HASH_ACCEPT :
> +
> + GEM_RX_UNICAST_HASH_ACCEPT;
> }
> }
> }
> --
> 2.19.1.windows.1
>
> ----------------------------------------------------------------------
> --------------------------------------------------------
> -----Original Message-----
> From: Edgar E. Iglesias [mailto:edgar.iglesias@gmail.com]
> Sent: Thursday, November 28, 2019 9:00 PM
> To: Wasim, Bilal <Bilal_Wasim@mentor.com>
> Cc: qemu-devel@nongnu.org; alistair@alistair23.me;
> peter.maydell@linaro.org; qemu-arm@nongnu.org
> Subject: Re: [PATCH] Updating the GEM MAC IP to properly filter out
> the multicast addresses
>
> On Thu, Nov 28, 2019 at 03:10:16PM +0000, Wasim, Bilal wrote:
> > [PATCH] Updating the GEM MAC IP to properly filter out the multicast
> > addresses. The current code makes a bad assumption that the
> > most-significant byte of the MAC address is used to determine if the
> > address is multicast or unicast, but in reality only a single bit is
> > used to determine this. This caused IPv6 to not work.. Fix is now in
> > place and has been tested with
> > ZCU102-A53 / IPv6 on a TAP interface. Works well..
>
> Hi Bilal,
>
> The fix looks right to me but I have a few comments.
>
> * Your patch seems a little wrongly formated.
> [PATCH] goes into the Subject line for example and you're missing path prefixes.
>
> Do a git log -- hw/net/cadence_gem.c to see examples on how it should look.
>
> * The patch will probably not pass checkpatch since you seem to have long lines.
>
> * We also need to update gem_receive_updatestats() to use the corrected macros.
>
> More inline:
>
> >
> > Signed-off-by: Bilal Wasim <bilal_wasim@mentor.com>
> > ---
> > hw/net/cadence_gem.c | 18 ++++++++++++------
> > 1 file changed, 12 insertions(+), 6 deletions(-)
> >
> > diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c index
> > b8be73d..f8bcbb3 100644
> > --- a/hw/net/cadence_gem.c
> > +++ b/hw/net/cadence_gem.c
> > @@ -315,6 +315,12 @@
> > #define GEM_MODID_VALUE 0x00020118
> > +/* IEEE has specified that the most significant bit of the most
> > +significant byte be used for
> > + * distinguishing between Unicast and Multicast addresses.
> > + * If its a 1, that means multicast, 0 means unicast. */
> > +#define IS_MULTICAST(address) (((address[0] & 0x01) == 0x01) ? 1 : 0)
>
>
>
> This can be simplified:
> #define IS_MULTICAST(address) (address[0] & 1)
>
> Actually, looking closer, we already have functions to do these checks in:
> include/net/eth.h
>
> static inline int is_multicast_ether_addr(const uint8_t *addr) static
> inline int is_broadcast_ether_addr(const uint8_t *addr) static inline
> int is_unicast_ether_addr(const uint8_t *addr)
>
>
>
> > +#define IS_UNICAST(address) (!IS_MULTICAST(address))
> > +
> > static inline uint64_t tx_desc_get_buffer(CadenceGEMState *s,
> > uint32_t
> > *desc) {
> > uint64_t ret = desc[0];
> > @@ -690,21 +696,21 @@ static int gem_mac_address_filter(CadenceGEMState *s, const uint8_t *packet)
> > }
> > /* Accept packets -w- hash match? */
> > - if ((packet[0] == 0x01 && (s->regs[GEM_NWCFG] & GEM_NWCFG_MCAST_HASH)) ||
> > - (packet[0] != 0x01 && (s->regs[GEM_NWCFG] & GEM_NWCFG_UCAST_HASH))) {
> > + if ((IS_MULTICAST(packet) && (s->regs[GEM_NWCFG] & GEM_NWCFG_MCAST_HASH)) ||
> > + (IS_UNICAST(packet) && (s->regs[GEM_NWCFG] & GEM_NWCFG_UCAST_HASH))) {
> > unsigned hash_index;
> > hash_index = calc_mac_hash(packet);
> > if (hash_index < 32) {
> > if (s->regs[GEM_HASHLO] & (1<<hash_index)) {
> > - return packet[0] == 0x01 ? GEM_RX_MULTICAST_HASH_ACCEPT :
> > - GEM_RX_UNICAST_HASH_ACCEPT;
> > + return IS_MULTICAST(packet) ? GEM_RX_MULTICAST_HASH_ACCEPT :
> > +
> > + GEM_RX_UNICAST_HASH_ACCEPT;
> > }
> > } else {
> > hash_index -= 32;
> > if (s->regs[GEM_HASHHI] & (1<<hash_index)) {
> > - return packet[0] == 0x01 ? GEM_RX_MULTICAST_HASH_ACCEPT :
> > - GEM_RX_UNICAST_HASH_ACCEPT;
> > + return IS_MULTICAST(packet) ? GEM_RX_MULTICAST_HASH_ACCEPT :
> > +
> > + GEM_RX_UNICAST_HASH_ACCEPT;
> > }
> > }
> > }
> > --
> > 2.9.3
> >
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH] Updating the GEM MAC IP to properly filter out the multicast addresses
2019-11-28 17:34 ` Wasim, Bilal
@ 2019-11-29 8:54 ` Wasim, Bilal
0 siblings, 0 replies; 7+ messages in thread
From: Wasim, Bilal @ 2019-11-29 8:54 UTC (permalink / raw)
To: Edgar E. Iglesias; +Cc: peter.maydell, alistair, qemu-devel, qemu-arm
git "send-email" doesn't really work well with my work account because the server considers it "less secure" and does NOT provide a way around it.. I've had to re-submit the patch from my secondary email...
-----Original Message-----
From: Qemu-arm [mailto:qemu-arm-bounces+bilal_wasim=mentor.com@nongnu.org] On Behalf Of Wasim, Bilal
Sent: Thursday, November 28, 2019 10:35 PM
To: Edgar E. Iglesias <edgar.iglesias@gmail.com>
Cc: peter.maydell@linaro.org; alistair@alistair23.me; qemu-devel@nongnu.org; qemu-arm@nongnu.org
Subject: RE: [PATCH] Updating the GEM MAC IP to properly filter out the multicast addresses
Thanks for the pointers.. I will incorporate all these changes and post an updated thread asap..
-----Original Message-----
From: Edgar E. Iglesias [mailto:edgar.iglesias@gmail.com]
Sent: Thursday, November 28, 2019 10:32 PM
To: Wasim, Bilal <Bilal_Wasim@mentor.com>
Cc: qemu-devel@nongnu.org; alistair@alistair23.me; peter.maydell@linaro.org; qemu-arm@nongnu.org
Subject: Re: [PATCH] Updating the GEM MAC IP to properly filter out the multicast addresses
On Thu, Nov 28, 2019 at 05:02:00PM +0000, Wasim, Bilal wrote:
> This was one of my first attempts, and so I was sure to miss something.. I've incorporated all the updates in this patch.. Let me know what you think about this..
>
> net/cadence_gem: Updating the GEM MAC IP to properly filter out the multicast addresses.
>
> The current code makes a bad assumption that the most-significant byte
> of the MAC address is used to determine if the address is multicast or
> unicast, but in reality only a single bit is used to determine this.
> This caused IPv6 to not work.. Fix is now in place and has been tested
> with ZCU102-A53 / IPv6 on a TAP interface. Works well..
Thanks Bilal,
This looks better but not quite there yet.
* You don't seem to be using git-send-email to post patches, try it, it will make life easier wrt to formatting. The patch should be in a separate email. The subject line should be the subject of the email.
git-format-patch and git-send-email will take care of that for you.
* You don't need to define IS_MULTICAST, you can directly use is_multicast_ether_addr() and friends.
* The patch still has long lines (longer than 80 chars) You can run scripts/checkpatch.pl on your commit before posting the patch.
Cheers,
Edgar
>
> Signed-off-by: Bilal Wasim <bilal_wasim@mentor.com>
> ---
> hw/net/cadence_gem.c | 21 ++++++++++++++-------
> 1 file changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c index
> b8be73dc55..98efb93f8a 100644
> --- a/hw/net/cadence_gem.c
> +++ b/hw/net/cadence_gem.c
> @@ -34,6 +34,7 @@
> #include "qemu/module.h"
> #include "sysemu/dma.h"
> #include "net/checksum.h"
> +#include "net/eth.h"
>
> #ifdef CADENCE_GEM_ERR_DEBUG
> #define DB_PRINT(...) do { \
> @@ -315,6 +316,12 @@
>
> #define GEM_MODID_VALUE 0x00020118
>
> +/* IEEE has specified that the most significant bit of the most
> +significant byte be used for
> + * distinguishing between Unicast and Multicast addresses.
> + * If its a 1, that means multicast, 0 means unicast. */
> +#define IS_MULTICAST(address) is_multicast_ether_addr(address)
> +#define IS_UNICAST(address) is_unicast_ether_addr(address)
> +
> static inline uint64_t tx_desc_get_buffer(CadenceGEMState *s,
> uint32_t *desc) {
> uint64_t ret = desc[0];
> @@ -601,7 +608,7 @@ static void gem_receive_updatestats(CadenceGEMState *s, const uint8_t *packet,
> }
>
> /* Error-free Multicast Frames counter */
> - if (packet[0] == 0x01) {
> + if (IS_MULTICAST(packet)) {
> s->regs[GEM_RXMULTICNT]++;
> }
>
> @@ -690,21 +697,21 @@ static int gem_mac_address_filter(CadenceGEMState *s, const uint8_t *packet)
> }
>
> /* Accept packets -w- hash match? */
> - if ((packet[0] == 0x01 && (s->regs[GEM_NWCFG] & GEM_NWCFG_MCAST_HASH)) ||
> - (packet[0] != 0x01 && (s->regs[GEM_NWCFG] & GEM_NWCFG_UCAST_HASH))) {
> + if ((IS_MULTICAST(packet) && (s->regs[GEM_NWCFG] & GEM_NWCFG_MCAST_HASH)) ||
> + (IS_UNICAST(packet) && (s->regs[GEM_NWCFG] & GEM_NWCFG_UCAST_HASH))) {
> unsigned hash_index;
>
> hash_index = calc_mac_hash(packet);
> if (hash_index < 32) {
> if (s->regs[GEM_HASHLO] & (1<<hash_index)) {
> - return packet[0] == 0x01 ? GEM_RX_MULTICAST_HASH_ACCEPT :
> - GEM_RX_UNICAST_HASH_ACCEPT;
> + return IS_MULTICAST(packet) ? GEM_RX_MULTICAST_HASH_ACCEPT :
> +
> + GEM_RX_UNICAST_HASH_ACCEPT;
> }
> } else {
> hash_index -= 32;
> if (s->regs[GEM_HASHHI] & (1<<hash_index)) {
> - return packet[0] == 0x01 ? GEM_RX_MULTICAST_HASH_ACCEPT :
> - GEM_RX_UNICAST_HASH_ACCEPT;
> + return IS_MULTICAST(packet) ? GEM_RX_MULTICAST_HASH_ACCEPT :
> +
> + GEM_RX_UNICAST_HASH_ACCEPT;
> }
> }
> }
> --
> 2.19.1.windows.1
>
> ----------------------------------------------------------------------
> --------------------------------------------------------
> -----Original Message-----
> From: Edgar E. Iglesias [mailto:edgar.iglesias@gmail.com]
> Sent: Thursday, November 28, 2019 9:00 PM
> To: Wasim, Bilal <Bilal_Wasim@mentor.com>
> Cc: qemu-devel@nongnu.org; alistair@alistair23.me;
> peter.maydell@linaro.org; qemu-arm@nongnu.org
> Subject: Re: [PATCH] Updating the GEM MAC IP to properly filter out
> the multicast addresses
>
> On Thu, Nov 28, 2019 at 03:10:16PM +0000, Wasim, Bilal wrote:
> > [PATCH] Updating the GEM MAC IP to properly filter out the multicast
> > addresses. The current code makes a bad assumption that the
> > most-significant byte of the MAC address is used to determine if the
> > address is multicast or unicast, but in reality only a single bit is
> > used to determine this. This caused IPv6 to not work.. Fix is now in
> > place and has been tested with
> > ZCU102-A53 / IPv6 on a TAP interface. Works well..
>
> Hi Bilal,
>
> The fix looks right to me but I have a few comments.
>
> * Your patch seems a little wrongly formated.
> [PATCH] goes into the Subject line for example and you're missing path prefixes.
>
> Do a git log -- hw/net/cadence_gem.c to see examples on how it should look.
>
> * The patch will probably not pass checkpatch since you seem to have long lines.
>
> * We also need to update gem_receive_updatestats() to use the corrected macros.
>
> More inline:
>
> >
> > Signed-off-by: Bilal Wasim <bilal_wasim@mentor.com>
> > ---
> > hw/net/cadence_gem.c | 18 ++++++++++++------
> > 1 file changed, 12 insertions(+), 6 deletions(-)
> >
> > diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c index
> > b8be73d..f8bcbb3 100644
> > --- a/hw/net/cadence_gem.c
> > +++ b/hw/net/cadence_gem.c
> > @@ -315,6 +315,12 @@
> > #define GEM_MODID_VALUE 0x00020118
> > +/* IEEE has specified that the most significant bit of the most
> > +significant byte be used for
> > + * distinguishing between Unicast and Multicast addresses.
> > + * If its a 1, that means multicast, 0 means unicast. */
> > +#define IS_MULTICAST(address) (((address[0] & 0x01) == 0x01) ? 1 : 0)
>
>
>
> This can be simplified:
> #define IS_MULTICAST(address) (address[0] & 1)
>
> Actually, looking closer, we already have functions to do these checks in:
> include/net/eth.h
>
> static inline int is_multicast_ether_addr(const uint8_t *addr) static
> inline int is_broadcast_ether_addr(const uint8_t *addr) static inline
> int is_unicast_ether_addr(const uint8_t *addr)
>
>
>
> > +#define IS_UNICAST(address) (!IS_MULTICAST(address))
> > +
> > static inline uint64_t tx_desc_get_buffer(CadenceGEMState *s,
> > uint32_t
> > *desc) {
> > uint64_t ret = desc[0];
> > @@ -690,21 +696,21 @@ static int gem_mac_address_filter(CadenceGEMState *s, const uint8_t *packet)
> > }
> > /* Accept packets -w- hash match? */
> > - if ((packet[0] == 0x01 && (s->regs[GEM_NWCFG] & GEM_NWCFG_MCAST_HASH)) ||
> > - (packet[0] != 0x01 && (s->regs[GEM_NWCFG] & GEM_NWCFG_UCAST_HASH))) {
> > + if ((IS_MULTICAST(packet) && (s->regs[GEM_NWCFG] & GEM_NWCFG_MCAST_HASH)) ||
> > + (IS_UNICAST(packet) && (s->regs[GEM_NWCFG] & GEM_NWCFG_UCAST_HASH))) {
> > unsigned hash_index;
> > hash_index = calc_mac_hash(packet);
> > if (hash_index < 32) {
> > if (s->regs[GEM_HASHLO] & (1<<hash_index)) {
> > - return packet[0] == 0x01 ? GEM_RX_MULTICAST_HASH_ACCEPT :
> > - GEM_RX_UNICAST_HASH_ACCEPT;
> > + return IS_MULTICAST(packet) ? GEM_RX_MULTICAST_HASH_ACCEPT :
> > +
> > + GEM_RX_UNICAST_HASH_ACCEPT;
> > }
> > } else {
> > hash_index -= 32;
> > if (s->regs[GEM_HASHHI] & (1<<hash_index)) {
> > - return packet[0] == 0x01 ? GEM_RX_MULTICAST_HASH_ACCEPT :
> > - GEM_RX_UNICAST_HASH_ACCEPT;
> > + return IS_MULTICAST(packet) ? GEM_RX_MULTICAST_HASH_ACCEPT :
> > +
> > + GEM_RX_UNICAST_HASH_ACCEPT;
> > }
> > }
> > }
> > --
> > 2.9.3
> >
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: RE: [PATCH] Updating the GEM MAC IP to properly filter out the multicast addresses
2019-11-28 17:02 ` Wasim, Bilal
2019-11-28 17:31 ` Edgar E. Iglesias
@ 2019-11-29 10:00 ` no-reply
1 sibling, 0 replies; 7+ messages in thread
From: no-reply @ 2019-11-29 10:00 UTC (permalink / raw)
To: Bilal_Wasim; +Cc: edgar.iglesias, alistair, qemu-devel, qemu-arm, peter.maydell
Patchew URL: https://patchew.org/QEMU/2b7a42487a5d4a258062bd209a0d13fa@SVR-IES-MBX-03.mgc.mentorg.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Subject: RE: [PATCH] Updating the GEM MAC IP to properly filter out the multicast addresses
Type: series
Message-id: 2b7a42487a5d4a258062bd209a0d13fa@SVR-IES-MBX-03.mgc.mentorg.com
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Switched to a new branch 'test'
da627f2 Updating the GEM MAC IP to properly filter out the multicast addresses
=== OUTPUT BEGIN ===
ERROR: line over 90 characters
#36: FILE: hw/net/cadence_gem.c:319:
+/* IEEE has specified that the most significant bit of the most significant byte be used for
WARNING: Block comments use a leading /* on a separate line
#36: FILE: hw/net/cadence_gem.c:319:
+/* IEEE has specified that the most significant bit of the most significant byte be used for
WARNING: Block comments use a trailing */ on a separate line
#38: FILE: hw/net/cadence_gem.c:321:
+ * If its a 1, that means multicast, 0 means unicast. */
total: 1 errors, 2 warnings, 54 lines checked
Commit da627f2a2cb5 (Updating the GEM MAC IP to properly filter out the multicast addresses) has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/2b7a42487a5d4a258062bd209a0d13fa@SVR-IES-MBX-03.mgc.mentorg.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-11-29 10:18 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-28 15:10 [PATCH] Updating the GEM MAC IP to properly filter out the multicast addresses Wasim, Bilal
2019-11-28 15:59 ` Edgar E. Iglesias
2019-11-28 17:02 ` Wasim, Bilal
2019-11-28 17:31 ` Edgar E. Iglesias
2019-11-28 17:34 ` Wasim, Bilal
2019-11-29 8:54 ` Wasim, Bilal
2019-11-29 10:00 ` no-reply
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).