Message ID | 20191210140617.16656-1-bilalwasim676@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [v3] net/imx_fec: Adding support for MAC filtering in the FEC IP implementation. | expand |
Hi Jason, Mark, Can you please review this patch.. --Bilal On Tue, 10 Dec 2019, 19:06 , <bilalwasim676@gmail.com> wrote: > From: bwasim <bilalwasim676@gmail.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 | 109 ++++++++++++++++++++++++++++++++++++++- > include/hw/net/imx_fec.h | 10 ++++ > 2 files changed, 118 insertions(+), 1 deletion(-) > > diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c > index bd99236864..d248f39fb0 100644 > --- a/hw/net/imx_fec.c > +++ b/hw/net/imx_fec.c > @@ -419,6 +419,79 @@ 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 = net_crc32_le(mac, mac_length); > + > + /* > + * 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) { > + /* Accept all packets in promiscuous mode (even if bc_rej is > set). */ > + return FEC_RX_PROMISCUOUS_ACCEPT; > + } > + > + /* Broadcast packet? */ > + if (!memcmp(packet, broadcast_addr, 6)) { > + /* Reject broadcast packets? */ > + if (s->regs[ENET_RCR] & ENET_RCR_BC_REJ) { > + return FEC_RX_REJECT; > + } > + /* Accept packets from broadcast address. */ > + return FEC_RX_BROADCAST_ACCEPT; > + } > + > + /* Accept packets -w- hash match? */ > + hash = calc_mac_hash(packet, 6); > + > + /* Accept packets -w- multicast hash match? */ > + if ((packet[0] & 0x01) == 0x01) { > + /* Computed hash matches GAUR / GALR register ? */ > + if (((hash < 32) && (s->regs[ENET_GALR] & (1 << hash))) > + || ((hash > 31) && (s->regs[ENET_GAUR] & (1 << (hash - > 32))))) { > + /* Accept multicast hash enabled address. */ > + return FEC_RX_MULTICAST_HASH_ACCEPT; > + } > + } else { > + /* Computed hash matches IAUR / IALR register ? */ > + if (((hash < 32) && (s->regs[ENET_IALR] & (1 << hash))) > + || ((hash > 31) && (s->regs[ENET_IAUR] & (1 << (hash - > 32))))) { > + /* Accept multicast hash enabled address. */ > + return FEC_RX_UNICAST_HASH_ACCEPT; > + } > + } > + > + /* 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))) { > + /* Accept packet because it matches my unicast address. */ > + return FEC_RX_UNICAST_ACCEPT; > + } > + > + /* Return -1 because we do NOT support MAC address filtering.. */ > + return FEC_RX_REJECT; > +} > + > static void imx_eth_update(IMXFECState *s) > { > /* > @@ -984,7 +1057,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 +1139,15 @@ 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 +1213,16 @@ 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) { > + /* The packet is destined for the "broadcast" address. */ > + bd.flags |= ENET_BD_BC; > + } else if (maf == FEC_RX_MULTICAST_HASH_ACCEPT) { > + /* The packet is destined for a "multicast" address. */ > + bd.flags |= ENET_BD_MC; > + } > + > imx_fec_write_bd(&bd, addr); > /* Advance to the next descriptor. */ > if ((bd.flags & ENET_BD_W) != 0) { > @@ -1159,8 +1249,15 @@ 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 +1351,16 @@ 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) { > + /* The packet is destined for the "broadcast" address. */ > + bd.flags |= ENET_BD_BC; > + } else if (maf == FEC_RX_MULTICAST_HASH_ACCEPT) { > + /* The packet is destined for a "multicast" address. */ > + bd.flags |= ENET_BD_MC; > + } > + > 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..f9cfcf6af5 100644 > --- a/include/hw/net/imx_fec.h > +++ b/include/hw/net/imx_fec.h > @@ -275,4 +275,14 @@ 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) > + > +#define FEC_HASH_BITS 6 /* #bits in hash */ > + > #endif > -- > 2.19.1.windows.1 > >
On 10/12/2019 14:06, bilalwasim676@gmail.com wrote: > From: bwasim <bilalwasim676@gmail.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 | 109 ++++++++++++++++++++++++++++++++++++++- > include/hw/net/imx_fec.h | 10 ++++ > 2 files changed, 118 insertions(+), 1 deletion(-) > > diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c > index bd99236864..d248f39fb0 100644 > --- a/hw/net/imx_fec.c > +++ b/hw/net/imx_fec.c > @@ -419,6 +419,79 @@ 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 = net_crc32_le(mac, mac_length); > + > + /* > + * 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; > +} Is it worth keeping this in a separate function? This appears to be a standard hash calculation and most other cards simply inline it like below: > +/* > + * 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) { > + /* Accept all packets in promiscuous mode (even if bc_rej is set). */ > + return FEC_RX_PROMISCUOUS_ACCEPT; > + } > + > + /* Broadcast packet? */ > + if (!memcmp(packet, broadcast_addr, 6)) { ETH_ALEN? > + /* Reject broadcast packets? */ > + if (s->regs[ENET_RCR] & ENET_RCR_BC_REJ) { > + return FEC_RX_REJECT; > + } > + /* Accept packets from broadcast address. */ > + return FEC_RX_BROADCAST_ACCEPT; > + } > + > + /* Accept packets -w- hash match? */ > + hash = calc_mac_hash(packet, 6); hash = net_crc32_le(buf, ETH_ALEN) >> 26 > + /* Accept packets -w- multicast hash match? */ > + if ((packet[0] & 0x01) == 0x01) { > + /* Computed hash matches GAUR / GALR register ? */ > + if (((hash < 32) && (s->regs[ENET_GALR] & (1 << hash))) > + || ((hash > 31) && (s->regs[ENET_GAUR] & (1 << (hash - 32))))) { > + /* Accept multicast hash enabled address. */ > + return FEC_RX_MULTICAST_HASH_ACCEPT; > + } > + } else { > + /* Computed hash matches IAUR / IALR register ? */ > + if (((hash < 32) && (s->regs[ENET_IALR] & (1 << hash))) > + || ((hash > 31) && (s->regs[ENET_IAUR] & (1 << (hash - 32))))) { > + /* Accept multicast hash enabled address. */ > + return FEC_RX_UNICAST_HASH_ACCEPT; > + } > + } > + > + /* 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))) { > + /* Accept packet because it matches my unicast address. */ > + return FEC_RX_UNICAST_ACCEPT; > + } > + > + /* Return -1 because we do NOT support MAC address filtering.. */ > + return FEC_RX_REJECT; > +} > + > static void imx_eth_update(IMXFECState *s) > { > /* > @@ -984,7 +1057,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 +1139,15 @@ 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 +1213,16 @@ 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) { > + /* The packet is destined for the "broadcast" address. */ > + bd.flags |= ENET_BD_BC; > + } else if (maf == FEC_RX_MULTICAST_HASH_ACCEPT) { > + /* The packet is destined for a "multicast" address. */ > + bd.flags |= ENET_BD_MC; > + } > + > imx_fec_write_bd(&bd, addr); > /* Advance to the next descriptor. */ > if ((bd.flags & ENET_BD_W) != 0) { > @@ -1159,8 +1249,15 @@ 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 +1351,16 @@ 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) { > + /* The packet is destined for the "broadcast" address. */ > + bd.flags |= ENET_BD_BC; > + } else if (maf == FEC_RX_MULTICAST_HASH_ACCEPT) { > + /* The packet is destined for a "multicast" address. */ > + bd.flags |= ENET_BD_MC; > + } > + > 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..f9cfcf6af5 100644 > --- a/include/hw/net/imx_fec.h > +++ b/include/hw/net/imx_fec.h > @@ -275,4 +275,14 @@ 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) > + > +#define FEC_HASH_BITS 6 /* #bits in hash */ If you inline the hash calculation as above then this line is no longer required. > #endif I've done some bits on QEMU networking however I'm not familiar with this particular device which limits me somewhat for further review. ATB, Mark.
diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c index bd99236864..d248f39fb0 100644 --- a/hw/net/imx_fec.c +++ b/hw/net/imx_fec.c @@ -419,6 +419,79 @@ 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 = net_crc32_le(mac, mac_length); + + /* + * 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) { + /* Accept all packets in promiscuous mode (even if bc_rej is set). */ + return FEC_RX_PROMISCUOUS_ACCEPT; + } + + /* Broadcast packet? */ + if (!memcmp(packet, broadcast_addr, 6)) { + /* Reject broadcast packets? */ + if (s->regs[ENET_RCR] & ENET_RCR_BC_REJ) { + return FEC_RX_REJECT; + } + /* Accept packets from broadcast address. */ + return FEC_RX_BROADCAST_ACCEPT; + } + + /* Accept packets -w- hash match? */ + hash = calc_mac_hash(packet, 6); + + /* Accept packets -w- multicast hash match? */ + if ((packet[0] & 0x01) == 0x01) { + /* Computed hash matches GAUR / GALR register ? */ + if (((hash < 32) && (s->regs[ENET_GALR] & (1 << hash))) + || ((hash > 31) && (s->regs[ENET_GAUR] & (1 << (hash - 32))))) { + /* Accept multicast hash enabled address. */ + return FEC_RX_MULTICAST_HASH_ACCEPT; + } + } else { + /* Computed hash matches IAUR / IALR register ? */ + if (((hash < 32) && (s->regs[ENET_IALR] & (1 << hash))) + || ((hash > 31) && (s->regs[ENET_IAUR] & (1 << (hash - 32))))) { + /* Accept multicast hash enabled address. */ + return FEC_RX_UNICAST_HASH_ACCEPT; + } + } + + /* 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))) { + /* Accept packet because it matches my unicast address. */ + return FEC_RX_UNICAST_ACCEPT; + } + + /* Return -1 because we do NOT support MAC address filtering.. */ + return FEC_RX_REJECT; +} + static void imx_eth_update(IMXFECState *s) { /* @@ -984,7 +1057,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 +1139,15 @@ 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 +1213,16 @@ 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) { + /* The packet is destined for the "broadcast" address. */ + bd.flags |= ENET_BD_BC; + } else if (maf == FEC_RX_MULTICAST_HASH_ACCEPT) { + /* The packet is destined for a "multicast" address. */ + bd.flags |= ENET_BD_MC; + } + imx_fec_write_bd(&bd, addr); /* Advance to the next descriptor. */ if ((bd.flags & ENET_BD_W) != 0) { @@ -1159,8 +1249,15 @@ 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 +1351,16 @@ 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) { + /* The packet is destined for the "broadcast" address. */ + bd.flags |= ENET_BD_BC; + } else if (maf == FEC_RX_MULTICAST_HASH_ACCEPT) { + /* The packet is destined for a "multicast" address. */ + bd.flags |= ENET_BD_MC; + } + 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..f9cfcf6af5 100644 --- a/include/hw/net/imx_fec.h +++ b/include/hw/net/imx_fec.h @@ -275,4 +275,14 @@ 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) + +#define FEC_HASH_BITS 6 /* #bits in hash */ + #endif