> On 17 Jul 2019, at 09:44, Martin Pieuchot <m...@openbsd.org> wrote:
>
> On 15/07/19(Mon) 15:29, Todd C. Miller wrote:
>> On Mon, 15 Jul 2019 18:21:28 -0300, Martin Pieuchot wrote:
>>
>>> We have many home brewed ways to check Ethernet addresses. This diff
>>> introduces two macros similar to ETHER_IS_MULTICAST() and make use of
>>> them in bridge(4) and generic Ethernet layer.
>>
>> I think you can also replace the ether_isbcast, ether_isequal and
>> ether_cmp macros in net/if_gre.c as well.
>
> Updated diff:
>
> - Introduce ETHER_IS() and convert the various ether_is_* in bpe(4) and
> gre(4) as pointed by millert@
>
> - Use ETHER_IS_BROADCAST() for compatibility with FreeBSD as pointed by
> emaste@
>
> Ok?
ok.
>
> Index: net/if_bpe.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if_bpe.c,v
> retrieving revision 1.7
> diff -u -p -r1.7 if_bpe.c
> --- net/if_bpe.c 21 May 2019 10:11:10 -0000 1.7
> +++ net/if_bpe.c 16 Jul 2019 23:37:52 -0000
> @@ -144,10 +144,6 @@ static struct bpe_tree bpe_interfaces =
> static struct rwlock bpe_lock = RWLOCK_INITIALIZER("bpeifs");
> static struct pool bpe_entry_pool;
>
> -#define ether_cmp(_a, _b) memcmp((_a), (_b), ETHER_ADDR_LEN)
> -#define ether_is_eq(_a, _b) (ether_cmp((_a), (_b)) == 0)
> -#define ether_is_bcast(_a) ether_is_eq((_a), etherbroadcastaddr)
> -
> void
> bpeattach(int count)
> {
> @@ -290,7 +286,7 @@ bpe_start(struct ifnet *ifp)
>
> beh = mtod(m, struct ether_header *);
>
> - if (ether_is_bcast(ceh->ether_dhost)) {
> + if (ETHER_IS_BROADCAST(ceh->ether_dhost)) {
> memcpy(beh->ether_dhost, sc->sc_group,
> sizeof(beh->ether_dhost));
> } else {
> @@ -839,7 +835,7 @@ bpe_input_map(struct bpe_softc *sc, cons
> be->be_age = time_uptime; /* only a little bit racy */
>
> if (be->be_type != BPE_ENTRY_DYNAMIC ||
> - ether_is_eq(ba, &be->be_b_da))
> + ETHER_IS_EQ(ba, &be->be_b_da))
> be = NULL;
> else
> refcnt_take(&be->be_refs);
> Index: net/if_bridge.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if_bridge.c,v
> retrieving revision 1.335
> diff -u -p -r1.335 if_bridge.c
> --- net/if_bridge.c 9 Jun 2019 17:42:16 -0000 1.335
> +++ net/if_bridge.c 16 Jul 2019 23:28:59 -0000
> @@ -944,10 +944,8 @@ bridgeintr_frame(struct ifnet *brifp, st
> * is not broadcast or multicast, record its address.
> */
> if ((bif->bif_flags & IFBIF_LEARNING) &&
> - (eh.ether_shost[0] & 1) == 0 &&
> - !(eh.ether_shost[0] == 0 && eh.ether_shost[1] == 0 &&
> - eh.ether_shost[2] == 0 && eh.ether_shost[3] == 0 &&
> - eh.ether_shost[4] == 0 && eh.ether_shost[5] == 0))
> + !ETHER_IS_MULTICAST(eh.ether_shost) &&
> + !ETHER_IS_ANYADDR(eh.ether_shost))
> bridge_rtupdate(sc, src, src_if, 0, IFBAF_DYNAMIC, m);
>
> if ((bif->bif_flags & IFBIF_STP) &&
> @@ -972,8 +970,7 @@ bridgeintr_frame(struct ifnet *brifp, st
> return;
> }
> } else {
> - if (memcmp(etherbroadcastaddr, eh.ether_dhost,
> - sizeof(etherbroadcastaddr)) == 0)
> + if (ETHER_IS_BROADCAST(eh.ether_dhost))
> m->m_flags |= M_BCAST;
> else
> m->m_flags |= M_MCAST;
> Index: net/if_ethersubr.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if_ethersubr.c,v
> retrieving revision 1.259
> diff -u -p -r1.259 if_ethersubr.c
> --- net/if_ethersubr.c 20 Feb 2019 00:03:15 -0000 1.259
> +++ net/if_ethersubr.c 16 Jul 2019 23:29:14 -0000
> @@ -377,8 +377,7 @@ ether_input(struct ifnet *ifp, struct mb
> goto dropanyway;
> }
>
> - if (memcmp(etherbroadcastaddr, eh->ether_dhost,
> - ETHER_ADDR_LEN) == 0)
> + if (ETHER_IS_BROADCAST(eh->ether_dhost))
> m->m_flags |= M_BCAST;
> else
> m->m_flags |= M_MCAST;
> Index: net/if_gre.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if_gre.c,v
> retrieving revision 1.150
> diff -u -p -r1.150 if_gre.c
> --- net/if_gre.c 23 Apr 2019 11:48:55 -0000 1.150
> +++ net/if_gre.c 16 Jul 2019 23:33:25 -0000
> @@ -353,9 +353,6 @@ struct mgre_tree mgre_tree = RBT_INITIAL
> /*
> * Ethernet GRE tunnels
> */
> -#define ether_cmp(_a, _b) memcmp((_a), (_b), ETHER_ADDR_LEN)
> -#define ether_isequal(_a, _b) (ether_cmp((_a), (_b)) == 0)
> -#define ether_isbcast(_e) ether_isequal((_e), etherbroadcastaddr)
>
> static struct mbuf *
> gre_ether_align(struct mbuf *, int);
> @@ -1455,7 +1452,7 @@ nvgre_input_map(struct nvgre_softc *sc,
> struct nvgre_entry *nv, nkey;
> int new = 0;
>
> - if (ether_isbcast(eh->ether_shost) ||
> + if (ETHER_IS_BROADCAST(eh->ether_shost) ||
> ETHER_IS_MULTICAST(eh->ether_shost))
> return;
>
> @@ -3863,7 +3860,7 @@ nvgre_start(struct ifnet *ifp)
> #endif
>
> eh = mtod(m0, struct ether_header *);
> - if (ether_isbcast(eh->ether_dhost))
> + if (ETHER_IS_BROADCAST(eh->ether_dhost))
> gateway = tunnel->t_dst;
> else {
> memcpy(&key.nv_dst, eh->ether_dhost,
> Index: netinet/if_ether.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/if_ether.c,v
> retrieving revision 1.239
> diff -u -p -r1.239 if_ether.c
> --- netinet/if_ether.c 13 Jun 2019 08:15:26 -0000 1.239
> +++ netinet/if_ether.c 16 Jul 2019 23:29:21 -0000
> @@ -516,8 +516,8 @@ in_arpinput(struct ifnet *ifp, struct mb
> sin.sin_len = sizeof(sin);
> sin.sin_family = AF_INET;
>
> - if (ETHER_IS_MULTICAST(&ea->arp_sha[0]) &&
> - !memcmp(ea->arp_sha, etherbroadcastaddr, sizeof(ea->arp_sha))) {
> + if (ETHER_IS_MULTICAST(ea->arp_sha) &&
> + ETHER_IS_BROADCAST(ea->arp_sha)) {
> inet_ntop(AF_INET, &isaddr, addr, sizeof(addr));
> log(LOG_ERR, "arp: ether address is broadcast for IP address "
> "%s!\n", addr);
> Index: netinet/if_ether.h
> ===================================================================
> RCS file: /cvs/src/sys/netinet/if_ether.h,v
> retrieving revision 1.75
> diff -u -p -r1.75 if_ether.h
> --- netinet/if_ether.h 5 Jul 2019 01:23:22 -0000 1.75
> +++ netinet/if_ether.h 16 Jul 2019 23:40:35 -0000
> @@ -108,6 +108,13 @@ struct ether_vlan_header {
> #include <net/ethertypes.h>
>
> #define ETHER_IS_MULTICAST(addr) (*(addr) & 0x01) /* is address
> mcast/bcast? */
> +#define ETHER_IS_BROADCAST(addr) \
> + (((addr)[0] & (addr)[1] & (addr)[2] & \
> + (addr)[3] & (addr)[4] & (addr)[5]) == 0xff)
> +#define ETHER_IS_ANYADDR(addr) \
> + (((addr)[0] | (addr)[1] | (addr)[2] | \
> + (addr)[3] | (addr)[4] | (addr)[5]) == 0x00)
> +#define ETHER_IS_EQ(a1, a2) (memcmp((a1), (a2), ETHER_ADDR_LEN) ==
> 0)
>
> #define ETHERMTU (ETHER_MAX_LEN - ETHER_HDR_LEN - ETHER_CRC_LEN)
> #define ETHERMIN (ETHER_MIN_LEN - ETHER_HDR_LEN - ETHER_CRC_LEN)
