[PATCH] Adding support for MAC filtering in the FEC IP implementation.

bilalwasim676@gmail.com posted 1 patch 4 years, 4 months ago
Test asan passed
Test checkpatch failed
Test FreeBSD passed
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Test docker-quick@centos7 passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20191207203450.6304-1-bilalwasim676@gmail.com
Maintainers: Peter Chubb <peter.chubb@nicta.com.au>, Peter Maydell <peter.maydell@linaro.org>, Jason Wang <jasowang@redhat.com>
There is a newer version of this series
hw/net/imx_fec.c         | 108 ++++++++++++++++++++++++++++++++++++++-
include/hw/net/imx_fec.h |  12 +++++
2 files changed, 119 insertions(+), 1 deletion(-)
[PATCH] Adding support for MAC filtering in the FEC IP implementation.
Posted by bilalwasim676@gmail.com 4 years, 4 months ago
From: Bilal Wasim <bilal_wasim@mentor.com>

This addition ensures that the IP does NOT boot up in
promiscuous mode by default, and so the software only receives the desired
packets(Unicast, Broadcast, Unicast / Multicast hashed) by default. The
software running on-top of QEMU can also modify these settings and disable
reception of broadcast frames or make the IP receive all packets (PROM mode).
This patch greatly reduces the number of packets received by the software
running on-top of the QEMU model. Tested with the armv7-a SABRE_LITE machine.
Testing included running a custom OS with IPv4 / IPv6 support. Hashing and
filtering of packets is tested to work well. Skeleton taken from the
CADENCE_GEM IP and hash generation algorithm from the Linux Kernel.

Signed-off-by: Bilal Wasim <bilalwasim676@gmail.com>
---
 hw/net/imx_fec.c         | 108 ++++++++++++++++++++++++++++++++++++++-
 include/hw/net/imx_fec.h |  12 +++++
 2 files changed, 119 insertions(+), 1 deletion(-)

diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
index bd99236864..dc39f1f597 100644
--- a/hw/net/imx_fec.c
+++ b/hw/net/imx_fec.c
@@ -419,6 +419,80 @@ static void imx_enet_write_bd(IMXENETBufDesc *bd, dma_addr_t addr)
     dma_memory_write(&address_space_memory, addr, bd, sizeof(*bd));
 }
 
+/*
+ * Calculate a FEC MAC Address hash index
+ */
+static unsigned calc_mac_hash(const uint8_t *mac, uint8_t mac_length)
+{
+    uint32_t crc = -1;
+    int i;
+
+    while (mac_length --) {
+        crc ^= *mac++;
+        for (i = 0; i < 8; i++) {
+            crc = (crc >> 1) ^ ((crc & 1) ? CRCPOLY_LE : 0);
+        }
+    }
+
+    /* only upper 6 bits (FEC_HASH_BITS) are used
+     * which point to specific bit in the hash registers
+     */
+    return ((crc >> (32 - FEC_HASH_BITS)) & 0x3f);
+}
+
+/*
+ * fec_mac_address_filter:
+ * Accept or reject this destination address?
+ */
+static int fec_mac_address_filter(IMXFECState *s, const uint8_t *packet)
+{
+    const uint8_t broadcast_addr[] = { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF };
+    uint32_t addr1, addr2;
+    uint8_t  hash;
+
+    /* Promiscuous mode? */
+    if (s->regs[ENET_RCR] & ENET_RCR_PROM) {
+        return (FEC_RX_PROMISCUOUS_ACCEPT); /* Accept all packets in promiscuous mode (even if bc_rej is set). */
+    }
+
+    /* Broadcast packet? */
+    if (!memcmp(packet, broadcast_addr, 6)) {
+        /* Reject broadcast packets? */
+        if (s->regs[ENET_RCR] & ENET_RCR_BC_REJ) {
+            return (FEC_RX_REJECT);
+        }
+        return (FEC_RX_BROADCAST_ACCEPT); /* Accept packets from broadcast address. */
+    }
+
+    /* Accept packets -w- hash match? */
+    hash = calc_mac_hash(packet, 6);
+
+    /* Accept packets -w- multicast hash match? */
+    if ((packet[0] & 0x01) == 0x01) {
+        /* See if the computed hash matches a set bit in either GAUR / GALR register. */
+        if (((hash < 32) && (s->regs[ENET_GALR] & (1 << hash)))
+                || ((hash > 31) && (s->regs[ENET_GAUR] & (1 << (hash-32))))) {
+            return (FEC_RX_MULTICAST_HASH_ACCEPT); /* Accept multicast hash enabled address. */
+        }
+    } else {
+        /* See if the computed hash matches a set bit in either IAUR / IALR register. */
+        if (((hash < 32) && (s->regs[ENET_IALR] & (1 << hash)))
+                || ((hash > 31) && (s->regs[ENET_IAUR] & (1 << (hash-32))))) {
+            return (FEC_RX_UNICAST_HASH_ACCEPT); /* Accept multicast hash enabled address. */
+        }
+    }
+
+    /* Match Unicast address. */
+    addr1  = g_htonl(s->regs[ENET_PALR]);
+    addr2  = g_htonl(s->regs[ENET_PAUR]);
+    if (!(memcmp(packet, (uint8_t *) &addr1, 4) || memcmp(packet+4, (uint8_t *) &addr2, 2))) {
+        return (FEC_RX_UNICAST_ACCEPT); /* Accept packet because it matches my unicast address. */
+    }
+
+    /* Return -1 because we do NOT support MAC address filtering.. */
+    return (FEC_RX_REJECT);
+}
+
 static void imx_eth_update(IMXFECState *s)
 {
     /*
@@ -984,7 +1058,7 @@ static void imx_eth_write(void *opaque, hwaddr offset, uint64_t value,
     case ENET_IALR:
     case ENET_GAUR:
     case ENET_GALR:
-        /* TODO: implement MAC hash filtering.  */
+        s->regs[index] |= value;
         break;
     case ENET_TFWR:
         if (s->is_fec) {
@@ -1066,8 +1140,16 @@ static ssize_t imx_fec_receive(NetClientState *nc, const uint8_t *buf,
     uint32_t buf_addr;
     uint8_t *crc_ptr;
     unsigned int buf_len;
+    int maf;
     size_t size = len;
 
+    /* Is this destination MAC address "for us" ? */
+    maf = fec_mac_address_filter(s, buf);
+    if (maf == FEC_RX_REJECT)
+    {
+        return (FEC_RX_REJECT);
+    }
+
     FEC_PRINTF("len %d\n", (int)size);
 
     if (!s->regs[ENET_RDAR]) {
@@ -1133,6 +1215,14 @@ static ssize_t imx_fec_receive(NetClientState *nc, const uint8_t *buf,
         } else {
             s->regs[ENET_EIR] |= ENET_INT_RXB;
         }
+
+        /* Update descriptor based on the "maf" flag. */
+        if (maf == FEC_RX_BROADCAST_ACCEPT) {
+            bd.flags |= ENET_BD_BC; /* The packet is destined for the "broadcast" address. */
+        } else if (maf == FEC_RX_MULTICAST_HASH_ACCEPT) {
+            bd.flags |= ENET_BD_MC; /* The packet is destined for a "multicast" address. */
+        }
+
         imx_fec_write_bd(&bd, addr);
         /* Advance to the next descriptor.  */
         if ((bd.flags & ENET_BD_W) != 0) {
@@ -1159,8 +1249,16 @@ static ssize_t imx_enet_receive(NetClientState *nc, const uint8_t *buf,
     uint8_t *crc_ptr;
     unsigned int buf_len;
     size_t size = len;
+    int maf;
     bool shift16 = s->regs[ENET_RACC] & ENET_RACC_SHIFT16;
 
+    /* Is this destination MAC address "for us" ? */
+    maf = fec_mac_address_filter(s, buf);
+    if (maf == FEC_RX_REJECT)
+    {
+        return (FEC_RX_REJECT);
+    }
+
     FEC_PRINTF("len %d\n", (int)size);
 
     if (!s->regs[ENET_RDAR]) {
@@ -1254,6 +1352,14 @@ static ssize_t imx_enet_receive(NetClientState *nc, const uint8_t *buf,
                 s->regs[ENET_EIR] |= ENET_INT_RXB;
             }
         }
+
+        /* Update descriptor based on the "maf" flag. */
+        if (maf == FEC_RX_BROADCAST_ACCEPT) {
+            bd.flags |= ENET_BD_BC; /* The packet is destined for the "broadcast" address. */
+        } else if (maf == FEC_RX_MULTICAST_HASH_ACCEPT) {
+            bd.flags |= ENET_BD_MC; /* The packet is destined for a "multicast" address. */
+        }
+
         imx_enet_write_bd(&bd, addr);
         /* Advance to the next descriptor.  */
         if ((bd.flags & ENET_BD_W) != 0) {
diff --git a/include/hw/net/imx_fec.h b/include/hw/net/imx_fec.h
index 7b3faa4019..d38c8fe0e8 100644
--- a/include/hw/net/imx_fec.h
+++ b/include/hw/net/imx_fec.h
@@ -275,4 +275,16 @@ typedef struct IMXFECState {
     uint8_t frame[ENET_MAX_FRAME_SIZE];
 } IMXFECState;
 
+/* FEC address filtering defines. */
+#define FEC_RX_REJECT                   (-1)
+#define FEC_RX_PROMISCUOUS_ACCEPT       (-2)
+#define FEC_RX_BROADCAST_ACCEPT         (-3)
+#define FEC_RX_MULTICAST_HASH_ACCEPT    (-4)
+#define FEC_RX_UNICAST_HASH_ACCEPT      (-5)
+#define FEC_RX_UNICAST_ACCEPT           (-6)
+
+/* FEC hash filtering defines.*/
+#define CRCPOLY_LE                      0xedb88320
+#define FEC_HASH_BITS                    6    /* #bits in hash */
+
 #endif
-- 
2.19.1.windows.1


Re: [PATCH] Adding support for MAC filtering in the FEC IP implementation.
Posted by no-reply@patchew.org 4 years, 4 months ago
Patchew URL: https://patchew.org/QEMU/20191207203450.6304-1-bilalwasim676@gmail.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [PATCH] Adding support for MAC filtering in the FEC IP implementation.
Type: series
Message-id: 20191207203450.6304-1-bilalwasim676@gmail.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'
19de63d Adding support for MAC filtering in the FEC IP implementation.

=== OUTPUT BEGIN ===
ERROR: space prohibited before that '--' (ctx:WxB)
#37: FILE: hw/net/imx_fec.c:430:
+    while (mac_length --) {
                       ^

WARNING: Block comments use a leading /* on a separate line
#44: FILE: hw/net/imx_fec.c:437:
+    /* only upper 6 bits (FEC_HASH_BITS) are used

ERROR: return is not a function, parentheses are not required
#47: FILE: hw/net/imx_fec.c:440:
+    return ((crc >> (32 - FEC_HASH_BITS)) & 0x3f);

ERROR: line over 90 characters
#62: FILE: hw/net/imx_fec.c:455:
+        return (FEC_RX_PROMISCUOUS_ACCEPT); /* Accept all packets in promiscuous mode (even if bc_rej is set). */

ERROR: return is not a function, parentheses are not required
#69: FILE: hw/net/imx_fec.c:462:
+            return (FEC_RX_REJECT);

WARNING: line over 80 characters
#71: FILE: hw/net/imx_fec.c:464:
+        return (FEC_RX_BROADCAST_ACCEPT); /* Accept packets from broadcast address. */

WARNING: line over 80 characters
#79: FILE: hw/net/imx_fec.c:472:
+        /* See if the computed hash matches a set bit in either GAUR / GALR register. */

ERROR: spaces required around that '-' (ctx:VxV)
#81: FILE: hw/net/imx_fec.c:474:
+                || ((hash > 31) && (s->regs[ENET_GAUR] & (1 << (hash-32))))) {
                                                                     ^

ERROR: line over 90 characters
#82: FILE: hw/net/imx_fec.c:475:
+            return (FEC_RX_MULTICAST_HASH_ACCEPT); /* Accept multicast hash enabled address. */

WARNING: line over 80 characters
#85: FILE: hw/net/imx_fec.c:478:
+        /* See if the computed hash matches a set bit in either IAUR / IALR register. */

ERROR: spaces required around that '-' (ctx:VxV)
#87: FILE: hw/net/imx_fec.c:480:
+                || ((hash > 31) && (s->regs[ENET_IAUR] & (1 << (hash-32))))) {
                                                                     ^

ERROR: line over 90 characters
#88: FILE: hw/net/imx_fec.c:481:
+            return (FEC_RX_UNICAST_HASH_ACCEPT); /* Accept multicast hash enabled address. */

ERROR: line over 90 characters
#95: FILE: hw/net/imx_fec.c:488:
+    if (!(memcmp(packet, (uint8_t *) &addr1, 4) || memcmp(packet+4, (uint8_t *) &addr2, 2))) {

ERROR: spaces required around that '+' (ctx:VxV)
#95: FILE: hw/net/imx_fec.c:488:
+    if (!(memcmp(packet, (uint8_t *) &addr1, 4) || memcmp(packet+4, (uint8_t *) &addr2, 2))) {
                                                                 ^

ERROR: line over 90 characters
#96: FILE: hw/net/imx_fec.c:489:
+        return (FEC_RX_UNICAST_ACCEPT); /* Accept packet because it matches my unicast address. */

ERROR: return is not a function, parentheses are not required
#100: FILE: hw/net/imx_fec.c:493:
+    return (FEC_RX_REJECT);

ERROR: that open brace { should be on the previous line
#124: FILE: hw/net/imx_fec.c:1148:
+    if (maf == FEC_RX_REJECT)
+    {

ERROR: return is not a function, parentheses are not required
#126: FILE: hw/net/imx_fec.c:1150:
+        return (FEC_RX_REJECT);

ERROR: line over 90 characters
#139: FILE: hw/net/imx_fec.c:1221:
+            bd.flags |= ENET_BD_BC; /* The packet is destined for the "broadcast" address. */

ERROR: line over 90 characters
#141: FILE: hw/net/imx_fec.c:1223:
+            bd.flags |= ENET_BD_MC; /* The packet is destined for a "multicast" address. */

ERROR: that open brace { should be on the previous line
#156: FILE: hw/net/imx_fec.c:1257:
+    if (maf == FEC_RX_REJECT)
+    {

ERROR: return is not a function, parentheses are not required
#158: FILE: hw/net/imx_fec.c:1259:
+        return (FEC_RX_REJECT);

ERROR: line over 90 characters
#171: FILE: hw/net/imx_fec.c:1358:
+            bd.flags |= ENET_BD_BC; /* The packet is destined for the "broadcast" address. */

ERROR: line over 90 characters
#173: FILE: hw/net/imx_fec.c:1360:
+            bd.flags |= ENET_BD_MC; /* The packet is destined for a "multicast" address. */

total: 20 errors, 4 warnings, 164 lines checked

Commit 19de63da8cde (Adding support for MAC filtering in the FEC IP implementation.) 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/20191207203450.6304-1-bilalwasim676@gmail.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com