* [PATCH] bridge: netfilter: spanning tree: Add masked_ether_addr_equal and neatening @ 2016-06-15 20:58 ` Joe Perches 0 siblings, 0 replies; 24+ messages in thread From: Joe Perches @ 2016-06-15 20:58 UTC (permalink / raw) To: Pablo Neira Ayuso, Patrick McHardy, Jozsef Kadlecsik Cc: Stephen Hemminger, David S. Miller, netfilter-devel, coreteam, bridge, netdev, linux-kernel There is code duplication of a masked ethernet address comparison here so make it a separate function instead. Miscellanea: o Neaten alignment of FWINV macro uses to make it clearer for the reader Signed-off-by: Joe Perches <joe@perches.com> --- This masked_ether_addr_equal function could go into etherdevice.h, but I don't see another use like it in kernel code. Is there one? net/bridge/netfilter/ebt_stp.c | 62 ++++++++++++++++++++++++------------------ 1 file changed, 35 insertions(+), 27 deletions(-) diff --git a/net/bridge/netfilter/ebt_stp.c b/net/bridge/netfilter/ebt_stp.c index e77f90b..46c3b5d 100644 --- a/net/bridge/netfilter/ebt_stp.c +++ b/net/bridge/netfilter/ebt_stp.c @@ -40,13 +40,25 @@ struct stp_config_pdu { #define NR16(p) (p[0] << 8 | p[1]) #define NR32(p) ((p[0] << 24) | (p[1] << 16) | (p[2] << 8) | p[3]) +static bool masked_ether_addr_equal(const u8 *addr1, const u8 *addr2, + const u8 *mask) +{ + int i; + + for (i = 0; i < ETH_ALEN; i++) { + if ((addr1[i] ^ addr2[i]) & mask[i]) + return false; + } + + return true; +} + static bool ebt_filter_config(const struct ebt_stp_info *info, const struct stp_config_pdu *stpc) { const struct ebt_stp_config_info *c; u16 v16; u32 v32; - int verdict, i; c = &info->config; if ((info->bitmask & EBT_STP_FLAGS) && @@ -54,66 +66,62 @@ static bool ebt_filter_config(const struct ebt_stp_info *info, return false; if (info->bitmask & EBT_STP_ROOTPRIO) { v16 = NR16(stpc->root); - if (FWINV(v16 < c->root_priol || - v16 > c->root_priou, EBT_STP_ROOTPRIO)) + if (FWINV(v16 < c->root_priol || v16 > c->root_priou, + EBT_STP_ROOTPRIO)) return false; } if (info->bitmask & EBT_STP_ROOTADDR) { - verdict = 0; - for (i = 0; i < 6; i++) - verdict |= (stpc->root[2+i] ^ c->root_addr[i]) & - c->root_addrmsk[i]; - if (FWINV(verdict != 0, EBT_STP_ROOTADDR)) + if (FWINV(!masked_ether_addr_equal(&stpc->root[2], c->root_addr, + c->root_addrmsk), + EBT_STP_ROOTADDR)) return false; } if (info->bitmask & EBT_STP_ROOTCOST) { v32 = NR32(stpc->root_cost); - if (FWINV(v32 < c->root_costl || - v32 > c->root_costu, EBT_STP_ROOTCOST)) + if (FWINV(v32 < c->root_costl || v32 > c->root_costu, + EBT_STP_ROOTCOST)) return false; } if (info->bitmask & EBT_STP_SENDERPRIO) { v16 = NR16(stpc->sender); - if (FWINV(v16 < c->sender_priol || - v16 > c->sender_priou, EBT_STP_SENDERPRIO)) + if (FWINV(v16 < c->sender_priol || v16 > c->sender_priou, + EBT_STP_SENDERPRIO)) return false; } if (info->bitmask & EBT_STP_SENDERADDR) { - verdict = 0; - for (i = 0; i < 6; i++) - verdict |= (stpc->sender[2+i] ^ c->sender_addr[i]) & - c->sender_addrmsk[i]; - if (FWINV(verdict != 0, EBT_STP_SENDERADDR)) + if (FWINV(!masked_ether_addr_equal(&stpc->sender[2], + c->sender_addr, + c->sender_addrmsk), + EBT_STP_SENDERADDR)) return false; } if (info->bitmask & EBT_STP_PORT) { v16 = NR16(stpc->port); - if (FWINV(v16 < c->portl || - v16 > c->portu, EBT_STP_PORT)) + if (FWINV(v16 < c->portl || v16 > c->portu, EBT_STP_PORT)) return false; } if (info->bitmask & EBT_STP_MSGAGE) { v16 = NR16(stpc->msg_age); - if (FWINV(v16 < c->msg_agel || - v16 > c->msg_ageu, EBT_STP_MSGAGE)) + if (FWINV(v16 < c->msg_agel || v16 > c->msg_ageu, + EBT_STP_MSGAGE)) return false; } if (info->bitmask & EBT_STP_MAXAGE) { v16 = NR16(stpc->max_age); - if (FWINV(v16 < c->max_agel || - v16 > c->max_ageu, EBT_STP_MAXAGE)) + if (FWINV(v16 < c->max_agel || v16 > c->max_ageu, + EBT_STP_MAXAGE)) return false; } if (info->bitmask & EBT_STP_HELLOTIME) { v16 = NR16(stpc->hello_time); - if (FWINV(v16 < c->hello_timel || - v16 > c->hello_timeu, EBT_STP_HELLOTIME)) + if (FWINV(v16 < c->hello_timel || v16 > c->hello_timeu, + EBT_STP_HELLOTIME)) return false; } if (info->bitmask & EBT_STP_FWDD) { v16 = NR16(stpc->forward_delay); - if (FWINV(v16 < c->forward_delayl || - v16 > c->forward_delayu, EBT_STP_FWDD)) + if (FWINV(v16 < c->forward_delayl || v16 > c->forward_delayu, + EBT_STP_FWDD)) return false; } return true; -- 2.8.0.rc4.16.g56331f8 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Bridge] [PATCH] bridge: netfilter: spanning tree: Add masked_ether_addr_equal and neatening @ 2016-06-15 20:58 ` Joe Perches 0 siblings, 0 replies; 24+ messages in thread From: Joe Perches @ 2016-06-15 20:58 UTC (permalink / raw) To: Pablo Neira Ayuso, Patrick McHardy, Jozsef Kadlecsik Cc: netdev, bridge, linux-kernel, coreteam, netfilter-devel, David S. Miller There is code duplication of a masked ethernet address comparison here so make it a separate function instead. Miscellanea: o Neaten alignment of FWINV macro uses to make it clearer for the reader Signed-off-by: Joe Perches <joe@perches.com> --- This masked_ether_addr_equal function could go into etherdevice.h, but I don't see another use like it in kernel code. Is there one? net/bridge/netfilter/ebt_stp.c | 62 ++++++++++++++++++++++++------------------ 1 file changed, 35 insertions(+), 27 deletions(-) diff --git a/net/bridge/netfilter/ebt_stp.c b/net/bridge/netfilter/ebt_stp.c index e77f90b..46c3b5d 100644 --- a/net/bridge/netfilter/ebt_stp.c +++ b/net/bridge/netfilter/ebt_stp.c @@ -40,13 +40,25 @@ struct stp_config_pdu { #define NR16(p) (p[0] << 8 | p[1]) #define NR32(p) ((p[0] << 24) | (p[1] << 16) | (p[2] << 8) | p[3]) +static bool masked_ether_addr_equal(const u8 *addr1, const u8 *addr2, + const u8 *mask) +{ + int i; + + for (i = 0; i < ETH_ALEN; i++) { + if ((addr1[i] ^ addr2[i]) & mask[i]) + return false; + } + + return true; +} + static bool ebt_filter_config(const struct ebt_stp_info *info, const struct stp_config_pdu *stpc) { const struct ebt_stp_config_info *c; u16 v16; u32 v32; - int verdict, i; c = &info->config; if ((info->bitmask & EBT_STP_FLAGS) && @@ -54,66 +66,62 @@ static bool ebt_filter_config(const struct ebt_stp_info *info, return false; if (info->bitmask & EBT_STP_ROOTPRIO) { v16 = NR16(stpc->root); - if (FWINV(v16 < c->root_priol || - v16 > c->root_priou, EBT_STP_ROOTPRIO)) + if (FWINV(v16 < c->root_priol || v16 > c->root_priou, + EBT_STP_ROOTPRIO)) return false; } if (info->bitmask & EBT_STP_ROOTADDR) { - verdict = 0; - for (i = 0; i < 6; i++) - verdict |= (stpc->root[2+i] ^ c->root_addr[i]) & - c->root_addrmsk[i]; - if (FWINV(verdict != 0, EBT_STP_ROOTADDR)) + if (FWINV(!masked_ether_addr_equal(&stpc->root[2], c->root_addr, + c->root_addrmsk), + EBT_STP_ROOTADDR)) return false; } if (info->bitmask & EBT_STP_ROOTCOST) { v32 = NR32(stpc->root_cost); - if (FWINV(v32 < c->root_costl || - v32 > c->root_costu, EBT_STP_ROOTCOST)) + if (FWINV(v32 < c->root_costl || v32 > c->root_costu, + EBT_STP_ROOTCOST)) return false; } if (info->bitmask & EBT_STP_SENDERPRIO) { v16 = NR16(stpc->sender); - if (FWINV(v16 < c->sender_priol || - v16 > c->sender_priou, EBT_STP_SENDERPRIO)) + if (FWINV(v16 < c->sender_priol || v16 > c->sender_priou, + EBT_STP_SENDERPRIO)) return false; } if (info->bitmask & EBT_STP_SENDERADDR) { - verdict = 0; - for (i = 0; i < 6; i++) - verdict |= (stpc->sender[2+i] ^ c->sender_addr[i]) & - c->sender_addrmsk[i]; - if (FWINV(verdict != 0, EBT_STP_SENDERADDR)) + if (FWINV(!masked_ether_addr_equal(&stpc->sender[2], + c->sender_addr, + c->sender_addrmsk), + EBT_STP_SENDERADDR)) return false; } if (info->bitmask & EBT_STP_PORT) { v16 = NR16(stpc->port); - if (FWINV(v16 < c->portl || - v16 > c->portu, EBT_STP_PORT)) + if (FWINV(v16 < c->portl || v16 > c->portu, EBT_STP_PORT)) return false; } if (info->bitmask & EBT_STP_MSGAGE) { v16 = NR16(stpc->msg_age); - if (FWINV(v16 < c->msg_agel || - v16 > c->msg_ageu, EBT_STP_MSGAGE)) + if (FWINV(v16 < c->msg_agel || v16 > c->msg_ageu, + EBT_STP_MSGAGE)) return false; } if (info->bitmask & EBT_STP_MAXAGE) { v16 = NR16(stpc->max_age); - if (FWINV(v16 < c->max_agel || - v16 > c->max_ageu, EBT_STP_MAXAGE)) + if (FWINV(v16 < c->max_agel || v16 > c->max_ageu, + EBT_STP_MAXAGE)) return false; } if (info->bitmask & EBT_STP_HELLOTIME) { v16 = NR16(stpc->hello_time); - if (FWINV(v16 < c->hello_timel || - v16 > c->hello_timeu, EBT_STP_HELLOTIME)) + if (FWINV(v16 < c->hello_timel || v16 > c->hello_timeu, + EBT_STP_HELLOTIME)) return false; } if (info->bitmask & EBT_STP_FWDD) { v16 = NR16(stpc->forward_delay); - if (FWINV(v16 < c->forward_delayl || - v16 > c->forward_delayu, EBT_STP_FWDD)) + if (FWINV(v16 < c->forward_delayl || v16 > c->forward_delayu, + EBT_STP_FWDD)) return false; } return true; -- 2.8.0.rc4.16.g56331f8 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH] bridge: netfilter: spanning tree: Add masked_ether_addr_equal and neatening 2016-06-15 20:58 ` [Bridge] " Joe Perches (?) @ 2016-06-16 6:04 ` Joe Perches -1 siblings, 0 replies; 24+ messages in thread From: Joe Perches @ 2016-06-16 6:04 UTC (permalink / raw) To: Pablo Neira Ayuso, Patrick McHardy, Jozsef Kadlecsik Cc: Stephen Hemminger, David S. Miller, netfilter-devel, coreteam, bridge, netdev, linux-kernel On Wed, 2016-06-15 at 13:58 -0700, Joe Perches wrote: > There is code duplication of a masked ethernet address comparison here > so make it a separate function instead. > > Miscellanea: > > o Neaten alignment of FWINV macro uses to make it clearer for the reader > > Signed-off-by: Joe Perches <joe@perches.com> > --- > > This masked_ether_addr_equal function could go into etherdevice.h, > but I don't see another use like it in kernel code. Is there one? Turns out there are at least a few more uses in bridge/netfilter net/bridge/netfilter/ebt_arp.c net/bridge/netfilter/ebtables.c Maybe this? --- >From 770261c682a745b8de663a5756a66cd00bb5b79b Mon Sep 17 00:00:00 2001 Message-Id: <770261c682a745b8de663a5756a66cd00bb5b79b.1466056695.git.joe@perches.com> From: Joe Perches <joe@perches.com> Date: Wed, 15 Jun 2016 13:45:54 -0700 Subject: [PATCH] etherdevice.h & bridge: netfilter: Add and use ether_addr_equal_masked There are code duplications of a masked ethernet address comparison here so make it a separate function instead. Miscellanea: o Neaten alignment of FWINV macro uses to make it clearer for the reader Signed-off-by: Joe Perches <joe@perches.com> --- include/linux/etherdevice.h | 22 ++++++++++++++++++ net/bridge/netfilter/ebt_arp.c | 17 +++++--------- net/bridge/netfilter/ebt_stp.c | 49 ++++++++++++++++++----------------------- net/bridge/netfilter/ebtables.c | 17 +++++--------- 4 files changed, 56 insertions(+), 49 deletions(-) diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h index 37ff4a6..942a24c 100644 --- a/include/linux/etherdevice.h +++ b/include/linux/etherdevice.h @@ -374,6 +374,28 @@ static inline bool ether_addr_equal_unaligned(const u8 *addr1, const u8 *addr2) } /** + * ether_addr_equal_masked - Compare two Ethernet addresses with a mask + * @addr1: Pointer to a six-byte array containing the 1st Ethernet address + * @addr2: Pointer to a six-byte array containing the 2nd Ethernet address + * @mask: Pointer to a six-byte array containing the Ethernet address bitmask + * + * Compare two Ethernet addresses with a mask, returns true if for every bit + * set in the bitmask the equivalent bits in the ethernet addresses are equal. + */ +static inline bool ether_addr_equal_masked(const u8 *addr1, const u8 *addr2, + const u8 *mask) +{ + int i; + + for (i = 0; i < ETH_ALEN; i++) { + if ((addr1[i] ^ addr2[i]) & mask[i]) + return false; + } + + return true; +} + +/** * is_etherdev_addr - Tell if given Ethernet address belongs to the device. * @dev: Pointer to a device structure * @addr: Pointer to a six-byte array containing the Ethernet address diff --git a/net/bridge/netfilter/ebt_arp.c b/net/bridge/netfilter/ebt_arp.c index cd457b8..cca0a89 100644 --- a/net/bridge/netfilter/ebt_arp.c +++ b/net/bridge/netfilter/ebt_arp.c @@ -65,7 +65,6 @@ ebt_arp_mt(const struct sk_buff *skb, struct xt_action_param *par) if (info->bitmask & (EBT_ARP_SRC_MAC | EBT_ARP_DST_MAC)) { const unsigned char *mp; unsigned char _mac[ETH_ALEN]; - uint8_t verdict, i; if (ah->ar_hln != ETH_ALEN || ah->ar_hrd != htons(ARPHRD_ETHER)) return false; @@ -74,11 +73,9 @@ ebt_arp_mt(const struct sk_buff *skb, struct xt_action_param *par) sizeof(_mac), &_mac); if (mp == NULL) return false; - verdict = 0; - for (i = 0; i < 6; i++) - verdict |= (mp[i] ^ info->smaddr[i]) & - info->smmsk[i]; - if (FWINV(verdict != 0, EBT_ARP_SRC_MAC)) + if (FWINV(!ether_addr_equal_masked(mp, info->smaddr, + info->smmsk), + EBT_ARP_SRC_MAC)) return false; } @@ -88,11 +85,9 @@ ebt_arp_mt(const struct sk_buff *skb, struct xt_action_param *par) sizeof(_mac), &_mac); if (mp == NULL) return false; - verdict = 0; - for (i = 0; i < 6; i++) - verdict |= (mp[i] ^ info->dmaddr[i]) & - info->dmmsk[i]; - if (FWINV(verdict != 0, EBT_ARP_DST_MAC)) + if (FWINV(!ether_addr_equal_masked(mp, info->dmaddr, + info->dmmsk), + EBT_ARP_DST_MAC)) return false; } } diff --git a/net/bridge/netfilter/ebt_stp.c b/net/bridge/netfilter/ebt_stp.c index e77f90b..45f73d5 100644 --- a/net/bridge/netfilter/ebt_stp.c +++ b/net/bridge/netfilter/ebt_stp.c @@ -46,7 +46,6 @@ static bool ebt_filter_config(const struct ebt_stp_info *info, const struct ebt_stp_config_info *c; u16 v16; u32 v32; - int verdict, i; c = &info->config; if ((info->bitmask & EBT_STP_FLAGS) && @@ -54,66 +53,62 @@ static bool ebt_filter_config(const struct ebt_stp_info *info, return false; if (info->bitmask & EBT_STP_ROOTPRIO) { v16 = NR16(stpc->root); - if (FWINV(v16 < c->root_priol || - v16 > c->root_priou, EBT_STP_ROOTPRIO)) + if (FWINV(v16 < c->root_priol || v16 > c->root_priou, + EBT_STP_ROOTPRIO)) return false; } if (info->bitmask & EBT_STP_ROOTADDR) { - verdict = 0; - for (i = 0; i < 6; i++) - verdict |= (stpc->root[2+i] ^ c->root_addr[i]) & - c->root_addrmsk[i]; - if (FWINV(verdict != 0, EBT_STP_ROOTADDR)) + if (FWINV(!ether_addr_equal_masked(&stpc->root[2], c->root_addr, + c->root_addrmsk), + EBT_STP_ROOTADDR)) return false; } if (info->bitmask & EBT_STP_ROOTCOST) { v32 = NR32(stpc->root_cost); - if (FWINV(v32 < c->root_costl || - v32 > c->root_costu, EBT_STP_ROOTCOST)) + if (FWINV(v32 < c->root_costl || v32 > c->root_costu, + EBT_STP_ROOTCOST)) return false; } if (info->bitmask & EBT_STP_SENDERPRIO) { v16 = NR16(stpc->sender); - if (FWINV(v16 < c->sender_priol || - v16 > c->sender_priou, EBT_STP_SENDERPRIO)) + if (FWINV(v16 < c->sender_priol || v16 > c->sender_priou, + EBT_STP_SENDERPRIO)) return false; } if (info->bitmask & EBT_STP_SENDERADDR) { - verdict = 0; - for (i = 0; i < 6; i++) - verdict |= (stpc->sender[2+i] ^ c->sender_addr[i]) & - c->sender_addrmsk[i]; - if (FWINV(verdict != 0, EBT_STP_SENDERADDR)) + if (FWINV(!ether_addr_equal_masked(&stpc->sender[2], + c->sender_addr, + c->sender_addrmsk), + EBT_STP_SENDERADDR)) return false; } if (info->bitmask & EBT_STP_PORT) { v16 = NR16(stpc->port); - if (FWINV(v16 < c->portl || - v16 > c->portu, EBT_STP_PORT)) + if (FWINV(v16 < c->portl || v16 > c->portu, EBT_STP_PORT)) return false; } if (info->bitmask & EBT_STP_MSGAGE) { v16 = NR16(stpc->msg_age); - if (FWINV(v16 < c->msg_agel || - v16 > c->msg_ageu, EBT_STP_MSGAGE)) + if (FWINV(v16 < c->msg_agel || v16 > c->msg_ageu, + EBT_STP_MSGAGE)) return false; } if (info->bitmask & EBT_STP_MAXAGE) { v16 = NR16(stpc->max_age); - if (FWINV(v16 < c->max_agel || - v16 > c->max_ageu, EBT_STP_MAXAGE)) + if (FWINV(v16 < c->max_agel || v16 > c->max_ageu, + EBT_STP_MAXAGE)) return false; } if (info->bitmask & EBT_STP_HELLOTIME) { v16 = NR16(stpc->hello_time); - if (FWINV(v16 < c->hello_timel || - v16 > c->hello_timeu, EBT_STP_HELLOTIME)) + if (FWINV(v16 < c->hello_timel || v16 > c->hello_timeu, + EBT_STP_HELLOTIME)) return false; } if (info->bitmask & EBT_STP_FWDD) { v16 = NR16(stpc->forward_delay); - if (FWINV(v16 < c->forward_delayl || - v16 > c->forward_delayu, EBT_STP_FWDD)) + if (FWINV(v16 < c->forward_delayl || v16 > c->forward_delayu, + EBT_STP_FWDD)) return false; } return true; diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c index 5a61f35..5721a25 100644 --- a/net/bridge/netfilter/ebtables.c +++ b/net/bridge/netfilter/ebtables.c @@ -130,7 +130,6 @@ ebt_basic_match(const struct ebt_entry *e, const struct sk_buff *skb, const struct ethhdr *h = eth_hdr(skb); const struct net_bridge_port *p; __be16 ethproto; - int verdict, i; if (skb_vlan_tag_present(skb)) ethproto = htons(ETH_P_8021Q); @@ -157,19 +156,15 @@ ebt_basic_match(const struct ebt_entry *e, const struct sk_buff *skb, return 1; if (e->bitmask & EBT_SOURCEMAC) { - verdict = 0; - for (i = 0; i < 6; i++) - verdict |= (h->h_source[i] ^ e->sourcemac[i]) & - e->sourcemsk[i]; - if (FWINV2(verdict != 0, EBT_ISOURCE)) + if (FWINV2(!ether_addr_equal_masked(h->h_source, + e->sourcemac, e->sourcemsk), + EBT_ISOURCE)) return 1; } if (e->bitmask & EBT_DESTMAC) { - verdict = 0; - for (i = 0; i < 6; i++) - verdict |= (h->h_dest[i] ^ e->destmac[i]) & - e->destmsk[i]; - if (FWINV2(verdict != 0, EBT_IDEST)) + if (FWINV2(!ether_addr_equal_masked(h->h_dest, + e->destmac, e->destmsk), + EBT_IDEST)) return 1; } return 0; -- 2.8.0.rc4.16.g56331f8 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [Bridge] [PATCH] bridge: netfilter: spanning tree: Add masked_ether_addr_equal and neatening @ 2016-06-16 6:04 ` Joe Perches 0 siblings, 0 replies; 24+ messages in thread From: Joe Perches @ 2016-06-16 6:04 UTC (permalink / raw) To: Pablo Neira Ayuso, Patrick McHardy, Jozsef Kadlecsik Cc: netdev, bridge, linux-kernel, coreteam, netfilter-devel, David S. Miller On Wed, 2016-06-15 at 13:58 -0700, Joe Perches wrote: > There is code duplication of a masked ethernet address comparison here > so make it a separate function instead. > > Miscellanea: > > o Neaten alignment of FWINV macro uses to make it clearer for the reader > > Signed-off-by: Joe Perches <joe@perches.com> > --- > > This masked_ether_addr_equal function could go into etherdevice.h, > but I don't see another use like it in kernel code. Is there one? Turns out there are at least a few more uses in bridge/netfilter net/bridge/netfilter/ebt_arp.c net/bridge/netfilter/ebtables.c Maybe this? --- From 770261c682a745b8de663a5756a66cd00bb5b79b Mon Sep 17 00:00:00 2001 Message-Id: <770261c682a745b8de663a5756a66cd00bb5b79b.1466056695.git.joe@perches.com> From: Joe Perches <joe@perches.com> Date: Wed, 15 Jun 2016 13:45:54 -0700 Subject: [PATCH] etherdevice.h & bridge: netfilter: Add and use ether_addr_equal_masked There are code duplications of a masked ethernet address comparison here so make it a separate function instead. Miscellanea: o Neaten alignment of FWINV macro uses to make it clearer for the reader Signed-off-by: Joe Perches <joe@perches.com> --- include/linux/etherdevice.h | 22 ++++++++++++++++++ net/bridge/netfilter/ebt_arp.c | 17 +++++--------- net/bridge/netfilter/ebt_stp.c | 49 ++++++++++++++++++----------------------- net/bridge/netfilter/ebtables.c | 17 +++++--------- 4 files changed, 56 insertions(+), 49 deletions(-) diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h index 37ff4a6..942a24c 100644 --- a/include/linux/etherdevice.h +++ b/include/linux/etherdevice.h @@ -374,6 +374,28 @@ static inline bool ether_addr_equal_unaligned(const u8 *addr1, const u8 *addr2) } /** + * ether_addr_equal_masked - Compare two Ethernet addresses with a mask + * @addr1: Pointer to a six-byte array containing the 1st Ethernet address + * @addr2: Pointer to a six-byte array containing the 2nd Ethernet address + * @mask: Pointer to a six-byte array containing the Ethernet address bitmask + * + * Compare two Ethernet addresses with a mask, returns true if for every bit + * set in the bitmask the equivalent bits in the ethernet addresses are equal. + */ +static inline bool ether_addr_equal_masked(const u8 *addr1, const u8 *addr2, + const u8 *mask) +{ + int i; + + for (i = 0; i < ETH_ALEN; i++) { + if ((addr1[i] ^ addr2[i]) & mask[i]) + return false; + } + + return true; +} + +/** * is_etherdev_addr - Tell if given Ethernet address belongs to the device. * @dev: Pointer to a device structure * @addr: Pointer to a six-byte array containing the Ethernet address diff --git a/net/bridge/netfilter/ebt_arp.c b/net/bridge/netfilter/ebt_arp.c index cd457b8..cca0a89 100644 --- a/net/bridge/netfilter/ebt_arp.c +++ b/net/bridge/netfilter/ebt_arp.c @@ -65,7 +65,6 @@ ebt_arp_mt(const struct sk_buff *skb, struct xt_action_param *par) if (info->bitmask & (EBT_ARP_SRC_MAC | EBT_ARP_DST_MAC)) { const unsigned char *mp; unsigned char _mac[ETH_ALEN]; - uint8_t verdict, i; if (ah->ar_hln != ETH_ALEN || ah->ar_hrd != htons(ARPHRD_ETHER)) return false; @@ -74,11 +73,9 @@ ebt_arp_mt(const struct sk_buff *skb, struct xt_action_param *par) sizeof(_mac), &_mac); if (mp == NULL) return false; - verdict = 0; - for (i = 0; i < 6; i++) - verdict |= (mp[i] ^ info->smaddr[i]) & - info->smmsk[i]; - if (FWINV(verdict != 0, EBT_ARP_SRC_MAC)) + if (FWINV(!ether_addr_equal_masked(mp, info->smaddr, + info->smmsk), + EBT_ARP_SRC_MAC)) return false; } @@ -88,11 +85,9 @@ ebt_arp_mt(const struct sk_buff *skb, struct xt_action_param *par) sizeof(_mac), &_mac); if (mp == NULL) return false; - verdict = 0; - for (i = 0; i < 6; i++) - verdict |= (mp[i] ^ info->dmaddr[i]) & - info->dmmsk[i]; - if (FWINV(verdict != 0, EBT_ARP_DST_MAC)) + if (FWINV(!ether_addr_equal_masked(mp, info->dmaddr, + info->dmmsk), + EBT_ARP_DST_MAC)) return false; } } diff --git a/net/bridge/netfilter/ebt_stp.c b/net/bridge/netfilter/ebt_stp.c index e77f90b..45f73d5 100644 --- a/net/bridge/netfilter/ebt_stp.c +++ b/net/bridge/netfilter/ebt_stp.c @@ -46,7 +46,6 @@ static bool ebt_filter_config(const struct ebt_stp_info *info, const struct ebt_stp_config_info *c; u16 v16; u32 v32; - int verdict, i; c = &info->config; if ((info->bitmask & EBT_STP_FLAGS) && @@ -54,66 +53,62 @@ static bool ebt_filter_config(const struct ebt_stp_info *info, return false; if (info->bitmask & EBT_STP_ROOTPRIO) { v16 = NR16(stpc->root); - if (FWINV(v16 < c->root_priol || - v16 > c->root_priou, EBT_STP_ROOTPRIO)) + if (FWINV(v16 < c->root_priol || v16 > c->root_priou, + EBT_STP_ROOTPRIO)) return false; } if (info->bitmask & EBT_STP_ROOTADDR) { - verdict = 0; - for (i = 0; i < 6; i++) - verdict |= (stpc->root[2+i] ^ c->root_addr[i]) & - c->root_addrmsk[i]; - if (FWINV(verdict != 0, EBT_STP_ROOTADDR)) + if (FWINV(!ether_addr_equal_masked(&stpc->root[2], c->root_addr, + c->root_addrmsk), + EBT_STP_ROOTADDR)) return false; } if (info->bitmask & EBT_STP_ROOTCOST) { v32 = NR32(stpc->root_cost); - if (FWINV(v32 < c->root_costl || - v32 > c->root_costu, EBT_STP_ROOTCOST)) + if (FWINV(v32 < c->root_costl || v32 > c->root_costu, + EBT_STP_ROOTCOST)) return false; } if (info->bitmask & EBT_STP_SENDERPRIO) { v16 = NR16(stpc->sender); - if (FWINV(v16 < c->sender_priol || - v16 > c->sender_priou, EBT_STP_SENDERPRIO)) + if (FWINV(v16 < c->sender_priol || v16 > c->sender_priou, + EBT_STP_SENDERPRIO)) return false; } if (info->bitmask & EBT_STP_SENDERADDR) { - verdict = 0; - for (i = 0; i < 6; i++) - verdict |= (stpc->sender[2+i] ^ c->sender_addr[i]) & - c->sender_addrmsk[i]; - if (FWINV(verdict != 0, EBT_STP_SENDERADDR)) + if (FWINV(!ether_addr_equal_masked(&stpc->sender[2], + c->sender_addr, + c->sender_addrmsk), + EBT_STP_SENDERADDR)) return false; } if (info->bitmask & EBT_STP_PORT) { v16 = NR16(stpc->port); - if (FWINV(v16 < c->portl || - v16 > c->portu, EBT_STP_PORT)) + if (FWINV(v16 < c->portl || v16 > c->portu, EBT_STP_PORT)) return false; } if (info->bitmask & EBT_STP_MSGAGE) { v16 = NR16(stpc->msg_age); - if (FWINV(v16 < c->msg_agel || - v16 > c->msg_ageu, EBT_STP_MSGAGE)) + if (FWINV(v16 < c->msg_agel || v16 > c->msg_ageu, + EBT_STP_MSGAGE)) return false; } if (info->bitmask & EBT_STP_MAXAGE) { v16 = NR16(stpc->max_age); - if (FWINV(v16 < c->max_agel || - v16 > c->max_ageu, EBT_STP_MAXAGE)) + if (FWINV(v16 < c->max_agel || v16 > c->max_ageu, + EBT_STP_MAXAGE)) return false; } if (info->bitmask & EBT_STP_HELLOTIME) { v16 = NR16(stpc->hello_time); - if (FWINV(v16 < c->hello_timel || - v16 > c->hello_timeu, EBT_STP_HELLOTIME)) + if (FWINV(v16 < c->hello_timel || v16 > c->hello_timeu, + EBT_STP_HELLOTIME)) return false; } if (info->bitmask & EBT_STP_FWDD) { v16 = NR16(stpc->forward_delay); - if (FWINV(v16 < c->forward_delayl || - v16 > c->forward_delayu, EBT_STP_FWDD)) + if (FWINV(v16 < c->forward_delayl || v16 > c->forward_delayu, + EBT_STP_FWDD)) return false; } return true; diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c index 5a61f35..5721a25 100644 --- a/net/bridge/netfilter/ebtables.c +++ b/net/bridge/netfilter/ebtables.c @@ -130,7 +130,6 @@ ebt_basic_match(const struct ebt_entry *e, const struct sk_buff *skb, const struct ethhdr *h = eth_hdr(skb); const struct net_bridge_port *p; __be16 ethproto; - int verdict, i; if (skb_vlan_tag_present(skb)) ethproto = htons(ETH_P_8021Q); @@ -157,19 +156,15 @@ ebt_basic_match(const struct ebt_entry *e, const struct sk_buff *skb, return 1; if (e->bitmask & EBT_SOURCEMAC) { - verdict = 0; - for (i = 0; i < 6; i++) - verdict |= (h->h_source[i] ^ e->sourcemac[i]) & - e->sourcemsk[i]; - if (FWINV2(verdict != 0, EBT_ISOURCE)) + if (FWINV2(!ether_addr_equal_masked(h->h_source, + e->sourcemac, e->sourcemsk), + EBT_ISOURCE)) return 1; } if (e->bitmask & EBT_DESTMAC) { - verdict = 0; - for (i = 0; i < 6; i++) - verdict |= (h->h_dest[i] ^ e->destmac[i]) & - e->destmsk[i]; - if (FWINV2(verdict != 0, EBT_IDEST)) + if (FWINV2(!ether_addr_equal_masked(h->h_dest, + e->destmac, e->destmsk), + EBT_IDEST)) return 1; } return 0; -- 2.8.0.rc4.16.g56331f8 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH] bridge: netfilter: spanning tree: Add masked_ether_addr_equal and neatening @ 2016-06-16 6:04 ` Joe Perches 0 siblings, 0 replies; 24+ messages in thread From: Joe Perches @ 2016-06-16 6:04 UTC (permalink / raw) To: Pablo Neira Ayuso, Patrick McHardy, Jozsef Kadlecsik Cc: Stephen Hemminger, David S. Miller, netfilter-devel, coreteam, bridge, netdev, linux-kernel On Wed, 2016-06-15 at 13:58 -0700, Joe Perches wrote: > There is code duplication of a masked ethernet address comparison here > so make it a separate function instead. > > Miscellanea: > > o Neaten alignment of FWINV macro uses to make it clearer for the reader > > Signed-off-by: Joe Perches <joe@perches.com> > --- > > This masked_ether_addr_equal function could go into etherdevice.h, > but I don't see another use like it in kernel code. Is there one? Turns out there are at least a few more uses in bridge/netfilter net/bridge/netfilter/ebt_arp.c net/bridge/netfilter/ebtables.c Maybe this? --- From 770261c682a745b8de663a5756a66cd00bb5b79b Mon Sep 17 00:00:00 2001 Message-Id: <770261c682a745b8de663a5756a66cd00bb5b79b.1466056695.git.joe@perches.com> From: Joe Perches <joe@perches.com> Date: Wed, 15 Jun 2016 13:45:54 -0700 Subject: [PATCH] etherdevice.h & bridge: netfilter: Add and use ether_addr_equal_masked There are code duplications of a masked ethernet address comparison here so make it a separate function instead. Miscellanea: o Neaten alignment of FWINV macro uses to make it clearer for the reader Signed-off-by: Joe Perches <joe@perches.com> --- include/linux/etherdevice.h | 22 ++++++++++++++++++ net/bridge/netfilter/ebt_arp.c | 17 +++++--------- net/bridge/netfilter/ebt_stp.c | 49 ++++++++++++++++++----------------------- net/bridge/netfilter/ebtables.c | 17 +++++--------- 4 files changed, 56 insertions(+), 49 deletions(-) diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h index 37ff4a6..942a24c 100644 --- a/include/linux/etherdevice.h +++ b/include/linux/etherdevice.h @@ -374,6 +374,28 @@ static inline bool ether_addr_equal_unaligned(const u8 *addr1, const u8 *addr2) } /** + * ether_addr_equal_masked - Compare two Ethernet addresses with a mask + * @addr1: Pointer to a six-byte array containing the 1st Ethernet address + * @addr2: Pointer to a six-byte array containing the 2nd Ethernet address + * @mask: Pointer to a six-byte array containing the Ethernet address bitmask + * + * Compare two Ethernet addresses with a mask, returns true if for every bit + * set in the bitmask the equivalent bits in the ethernet addresses are equal. + */ +static inline bool ether_addr_equal_masked(const u8 *addr1, const u8 *addr2, + const u8 *mask) +{ + int i; + + for (i = 0; i < ETH_ALEN; i++) { + if ((addr1[i] ^ addr2[i]) & mask[i]) + return false; + } + + return true; +} + +/** * is_etherdev_addr - Tell if given Ethernet address belongs to the device. * @dev: Pointer to a device structure * @addr: Pointer to a six-byte array containing the Ethernet address diff --git a/net/bridge/netfilter/ebt_arp.c b/net/bridge/netfilter/ebt_arp.c index cd457b8..cca0a89 100644 --- a/net/bridge/netfilter/ebt_arp.c +++ b/net/bridge/netfilter/ebt_arp.c @@ -65,7 +65,6 @@ ebt_arp_mt(const struct sk_buff *skb, struct xt_action_param *par) if (info->bitmask & (EBT_ARP_SRC_MAC | EBT_ARP_DST_MAC)) { const unsigned char *mp; unsigned char _mac[ETH_ALEN]; - uint8_t verdict, i; if (ah->ar_hln != ETH_ALEN || ah->ar_hrd != htons(ARPHRD_ETHER)) return false; @@ -74,11 +73,9 @@ ebt_arp_mt(const struct sk_buff *skb, struct xt_action_param *par) sizeof(_mac), &_mac); if (mp == NULL) return false; - verdict = 0; - for (i = 0; i < 6; i++) - verdict |= (mp[i] ^ info->smaddr[i]) & - info->smmsk[i]; - if (FWINV(verdict != 0, EBT_ARP_SRC_MAC)) + if (FWINV(!ether_addr_equal_masked(mp, info->smaddr, + info->smmsk), + EBT_ARP_SRC_MAC)) return false; } @@ -88,11 +85,9 @@ ebt_arp_mt(const struct sk_buff *skb, struct xt_action_param *par) sizeof(_mac), &_mac); if (mp == NULL) return false; - verdict = 0; - for (i = 0; i < 6; i++) - verdict |= (mp[i] ^ info->dmaddr[i]) & - info->dmmsk[i]; - if (FWINV(verdict != 0, EBT_ARP_DST_MAC)) + if (FWINV(!ether_addr_equal_masked(mp, info->dmaddr, + info->dmmsk), + EBT_ARP_DST_MAC)) return false; } } diff --git a/net/bridge/netfilter/ebt_stp.c b/net/bridge/netfilter/ebt_stp.c index e77f90b..45f73d5 100644 --- a/net/bridge/netfilter/ebt_stp.c +++ b/net/bridge/netfilter/ebt_stp.c @@ -46,7 +46,6 @@ static bool ebt_filter_config(const struct ebt_stp_info *info, const struct ebt_stp_config_info *c; u16 v16; u32 v32; - int verdict, i; c = &info->config; if ((info->bitmask & EBT_STP_FLAGS) && @@ -54,66 +53,62 @@ static bool ebt_filter_config(const struct ebt_stp_info *info, return false; if (info->bitmask & EBT_STP_ROOTPRIO) { v16 = NR16(stpc->root); - if (FWINV(v16 < c->root_priol || - v16 > c->root_priou, EBT_STP_ROOTPRIO)) + if (FWINV(v16 < c->root_priol || v16 > c->root_priou, + EBT_STP_ROOTPRIO)) return false; } if (info->bitmask & EBT_STP_ROOTADDR) { - verdict = 0; - for (i = 0; i < 6; i++) - verdict |= (stpc->root[2+i] ^ c->root_addr[i]) & - c->root_addrmsk[i]; - if (FWINV(verdict != 0, EBT_STP_ROOTADDR)) + if (FWINV(!ether_addr_equal_masked(&stpc->root[2], c->root_addr, + c->root_addrmsk), + EBT_STP_ROOTADDR)) return false; } if (info->bitmask & EBT_STP_ROOTCOST) { v32 = NR32(stpc->root_cost); - if (FWINV(v32 < c->root_costl || - v32 > c->root_costu, EBT_STP_ROOTCOST)) + if (FWINV(v32 < c->root_costl || v32 > c->root_costu, + EBT_STP_ROOTCOST)) return false; } if (info->bitmask & EBT_STP_SENDERPRIO) { v16 = NR16(stpc->sender); - if (FWINV(v16 < c->sender_priol || - v16 > c->sender_priou, EBT_STP_SENDERPRIO)) + if (FWINV(v16 < c->sender_priol || v16 > c->sender_priou, + EBT_STP_SENDERPRIO)) return false; } if (info->bitmask & EBT_STP_SENDERADDR) { - verdict = 0; - for (i = 0; i < 6; i++) - verdict |= (stpc->sender[2+i] ^ c->sender_addr[i]) & - c->sender_addrmsk[i]; - if (FWINV(verdict != 0, EBT_STP_SENDERADDR)) + if (FWINV(!ether_addr_equal_masked(&stpc->sender[2], + c->sender_addr, + c->sender_addrmsk), + EBT_STP_SENDERADDR)) return false; } if (info->bitmask & EBT_STP_PORT) { v16 = NR16(stpc->port); - if (FWINV(v16 < c->portl || - v16 > c->portu, EBT_STP_PORT)) + if (FWINV(v16 < c->portl || v16 > c->portu, EBT_STP_PORT)) return false; } if (info->bitmask & EBT_STP_MSGAGE) { v16 = NR16(stpc->msg_age); - if (FWINV(v16 < c->msg_agel || - v16 > c->msg_ageu, EBT_STP_MSGAGE)) + if (FWINV(v16 < c->msg_agel || v16 > c->msg_ageu, + EBT_STP_MSGAGE)) return false; } if (info->bitmask & EBT_STP_MAXAGE) { v16 = NR16(stpc->max_age); - if (FWINV(v16 < c->max_agel || - v16 > c->max_ageu, EBT_STP_MAXAGE)) + if (FWINV(v16 < c->max_agel || v16 > c->max_ageu, + EBT_STP_MAXAGE)) return false; } if (info->bitmask & EBT_STP_HELLOTIME) { v16 = NR16(stpc->hello_time); - if (FWINV(v16 < c->hello_timel || - v16 > c->hello_timeu, EBT_STP_HELLOTIME)) + if (FWINV(v16 < c->hello_timel || v16 > c->hello_timeu, + EBT_STP_HELLOTIME)) return false; } if (info->bitmask & EBT_STP_FWDD) { v16 = NR16(stpc->forward_delay); - if (FWINV(v16 < c->forward_delayl || - v16 > c->forward_delayu, EBT_STP_FWDD)) + if (FWINV(v16 < c->forward_delayl || v16 > c->forward_delayu, + EBT_STP_FWDD)) return false; } return true; diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c index 5a61f35..5721a25 100644 --- a/net/bridge/netfilter/ebtables.c +++ b/net/bridge/netfilter/ebtables.c @@ -130,7 +130,6 @@ ebt_basic_match(const struct ebt_entry *e, const struct sk_buff *skb, const struct ethhdr *h = eth_hdr(skb); const struct net_bridge_port *p; __be16 ethproto; - int verdict, i; if (skb_vlan_tag_present(skb)) ethproto = htons(ETH_P_8021Q); @@ -157,19 +156,15 @@ ebt_basic_match(const struct ebt_entry *e, const struct sk_buff *skb, return 1; if (e->bitmask & EBT_SOURCEMAC) { - verdict = 0; - for (i = 0; i < 6; i++) - verdict |= (h->h_source[i] ^ e->sourcemac[i]) & - e->sourcemsk[i]; - if (FWINV2(verdict != 0, EBT_ISOURCE)) + if (FWINV2(!ether_addr_equal_masked(h->h_source, + e->sourcemac, e->sourcemsk), + EBT_ISOURCE)) return 1; } if (e->bitmask & EBT_DESTMAC) { - verdict = 0; - for (i = 0; i < 6; i++) - verdict |= (h->h_dest[i] ^ e->destmac[i]) & - e->destmsk[i]; - if (FWINV2(verdict != 0, EBT_IDEST)) + if (FWINV2(!ether_addr_equal_masked(h->h_dest, + e->destmac, e->destmsk), + EBT_IDEST)) return 1; } return 0; -- 2.8.0.rc4.16.g56331f8 -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH] bridge: netfilter: spanning tree: Add masked_ether_addr_equal and neatening 2016-06-15 20:58 ` [Bridge] " Joe Perches (?) @ 2016-06-23 17:36 ` Pablo Neira Ayuso -1 siblings, 0 replies; 24+ messages in thread From: Pablo Neira Ayuso @ 2016-06-23 17:36 UTC (permalink / raw) To: Joe Perches Cc: Patrick McHardy, Jozsef Kadlecsik, Stephen Hemminger, David S. Miller, netfilter-devel, coreteam, bridge, netdev, linux-kernel On Wed, Jun 15, 2016 at 01:58:45PM -0700, Joe Perches wrote: > There is code duplication of a masked ethernet address comparison here > so make it a separate function instead. > > Miscellanea: > > o Neaten alignment of FWINV macro uses to make it clearer for the reader Applied, thanks. > Signed-off-by: Joe Perches <joe@perches.com> > --- > > This masked_ether_addr_equal function could go into etherdevice.h, > but I don't see another use like it in kernel code. Is there one? This is specific of iptables, not even nftables would use this. So I would keep this in the iptables tree. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Bridge] [PATCH] bridge: netfilter: spanning tree: Add masked_ether_addr_equal and neatening @ 2016-06-23 17:36 ` Pablo Neira Ayuso 0 siblings, 0 replies; 24+ messages in thread From: Pablo Neira Ayuso @ 2016-06-23 17:36 UTC (permalink / raw) To: Joe Perches Cc: netdev, bridge, linux-kernel, Patrick McHardy, coreteam, netfilter-devel, Jozsef Kadlecsik, David S. Miller On Wed, Jun 15, 2016 at 01:58:45PM -0700, Joe Perches wrote: > There is code duplication of a masked ethernet address comparison here > so make it a separate function instead. > > Miscellanea: > > o Neaten alignment of FWINV macro uses to make it clearer for the reader Applied, thanks. > Signed-off-by: Joe Perches <joe@perches.com> > --- > > This masked_ether_addr_equal function could go into etherdevice.h, > but I don't see another use like it in kernel code. Is there one? This is specific of iptables, not even nftables would use this. So I would keep this in the iptables tree. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] bridge: netfilter: spanning tree: Add masked_ether_addr_equal and neatening @ 2016-06-23 17:36 ` Pablo Neira Ayuso 0 siblings, 0 replies; 24+ messages in thread From: Pablo Neira Ayuso @ 2016-06-23 17:36 UTC (permalink / raw) To: Joe Perches Cc: netdev, bridge, linux-kernel, Patrick McHardy, coreteam, netfilter-devel, Jozsef Kadlecsik, David S. Miller On Wed, Jun 15, 2016 at 01:58:45PM -0700, Joe Perches wrote: > There is code duplication of a masked ethernet address comparison here > so make it a separate function instead. > > Miscellanea: > > o Neaten alignment of FWINV macro uses to make it clearer for the reader Applied, thanks. > Signed-off-by: Joe Perches <joe@perches.com> > --- > > This masked_ether_addr_equal function could go into etherdevice.h, > but I don't see another use like it in kernel code. Is there one? This is specific of iptables, not even nftables would use this. So I would keep this in the iptables tree. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] bridge: netfilter: spanning tree: Add masked_ether_addr_equal and neatening 2016-06-23 17:36 ` Pablo Neira Ayuso @ 2016-06-23 19:00 ` Joe Perches -1 siblings, 0 replies; 24+ messages in thread From: Joe Perches @ 2016-06-23 19:00 UTC (permalink / raw) To: Pablo Neira Ayuso Cc: Patrick McHardy, Jozsef Kadlecsik, Stephen Hemminger, David S. Miller, netfilter-devel, coreteam, bridge, netdev, linux-kernel On Thu, 2016-06-23 at 19:36 +0200, Pablo Neira Ayuso wrote: > On Wed, Jun 15, 2016 at 01:58:45PM -0700, Joe Perches wrote: > > > > There is code duplication of a masked ethernet address comparison here > > so make it a separate function instead. > > > > Miscellanea: > > > > o Neaten alignment of FWINV macro uses to make it clearer for the reader > Applied, thanks. > > > > > Signed-off-by: Joe Perches <joe@perches.com> > > --- > > > > This masked_ether_addr_equal function could go into etherdevice.h, > > but I don't see another use like it in kernel code. Is there one? > This is specific of iptables, not even nftables would use this. So I > would keep this in the iptables tree. Did you see the other patch that adds a generic ether_addr_equal_masked() and uses it in a few more files? ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Bridge] [PATCH] bridge: netfilter: spanning tree: Add masked_ether_addr_equal and neatening @ 2016-06-23 19:00 ` Joe Perches 0 siblings, 0 replies; 24+ messages in thread From: Joe Perches @ 2016-06-23 19:00 UTC (permalink / raw) To: Pablo Neira Ayuso Cc: netdev, bridge, linux-kernel, Patrick McHardy, coreteam, netfilter-devel, Jozsef Kadlecsik, David S. Miller On Thu, 2016-06-23 at 19:36 +0200, Pablo Neira Ayuso wrote: > On Wed, Jun 15, 2016 at 01:58:45PM -0700, Joe Perches wrote: > > > > There is code duplication of a masked ethernet address comparison here > > so make it a separate function instead. > > > > Miscellanea: > > > > o Neaten alignment of FWINV macro uses to make it clearer for the reader > Applied, thanks. > > > > > Signed-off-by: Joe Perches <joe@perches.com> > > --- > > > > This masked_ether_addr_equal function could go into etherdevice.h, > > but I don't see another use like it in kernel code. Is there one? > This is specific of iptables, not even nftables would use this. So I > would keep this in the iptables tree. Did you see the other patch that adds a generic ether_addr_equal_masked() and uses it in a few more files? ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] bridge: netfilter: spanning tree: Add masked_ether_addr_equal and neatening 2016-06-23 19:00 ` [Bridge] " Joe Perches (?) @ 2016-06-24 8:51 ` Pablo Neira Ayuso -1 siblings, 0 replies; 24+ messages in thread From: Pablo Neira Ayuso @ 2016-06-24 8:51 UTC (permalink / raw) To: Joe Perches Cc: Patrick McHardy, Jozsef Kadlecsik, Stephen Hemminger, David S. Miller, netfilter-devel, coreteam, bridge, netdev, linux-kernel On Thu, Jun 23, 2016 at 12:00:00PM -0700, Joe Perches wrote: > On Thu, 2016-06-23 at 19:36 +0200, Pablo Neira Ayuso wrote: > > On Wed, Jun 15, 2016 at 01:58:45PM -0700, Joe Perches wrote: > > > > > > There is code duplication of a masked ethernet address comparison here > > > so make it a separate function instead. > > > > > > Miscellanea: > > > > > > o Neaten alignment of FWINV macro uses to make it clearer for the reader > > Applied, thanks. > > > > > > > > Signed-off-by: Joe Perches <joe@perches.com> > > > --- > > > > > > This masked_ether_addr_equal function could go into etherdevice.h, > > > but I don't see another use like it in kernel code. Is there one? > > > > This is specific of iptables, not even nftables would use this. So I > > would keep this in the iptables tree. > > Did you see the other patch that adds a generic > ether_addr_equal_masked() and uses it in a few > more files? You mean this one: http://patchwork.ozlabs.org/patch/636208/ OK, so I'll toss the previous and will take this one instead. As I said my opinion is that ether_addr_equal_masked() is only required by netfilter, but thinking it well I don't really mind in what header this function is placed given that these are our internal headers. Thanks. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Bridge] [PATCH] bridge: netfilter: spanning tree: Add masked_ether_addr_equal and neatening @ 2016-06-24 8:51 ` Pablo Neira Ayuso 0 siblings, 0 replies; 24+ messages in thread From: Pablo Neira Ayuso @ 2016-06-24 8:51 UTC (permalink / raw) To: Joe Perches Cc: netdev, bridge, linux-kernel, Patrick McHardy, coreteam, netfilter-devel, Jozsef Kadlecsik, David S. Miller On Thu, Jun 23, 2016 at 12:00:00PM -0700, Joe Perches wrote: > On Thu, 2016-06-23 at 19:36 +0200, Pablo Neira Ayuso wrote: > > On Wed, Jun 15, 2016 at 01:58:45PM -0700, Joe Perches wrote: > > > > > > There is code duplication of a masked ethernet address comparison here > > > so make it a separate function instead. > > > > > > Miscellanea: > > > > > > o Neaten alignment of FWINV macro uses to make it clearer for the reader > > Applied, thanks. > > > > > > > > Signed-off-by: Joe Perches <joe@perches.com> > > > --- > > > > > > This masked_ether_addr_equal function could go into etherdevice.h, > > > but I don't see another use like it in kernel code. Is there one? > > > > This is specific of iptables, not even nftables would use this. So I > > would keep this in the iptables tree. > > Did you see the other patch that adds a generic > ether_addr_equal_masked() and uses it in a few > more files? You mean this one: http://patchwork.ozlabs.org/patch/636208/ OK, so I'll toss the previous and will take this one instead. As I said my opinion is that ether_addr_equal_masked() is only required by netfilter, but thinking it well I don't really mind in what header this function is placed given that these are our internal headers. Thanks. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] bridge: netfilter: spanning tree: Add masked_ether_addr_equal and neatening @ 2016-06-24 8:51 ` Pablo Neira Ayuso 0 siblings, 0 replies; 24+ messages in thread From: Pablo Neira Ayuso @ 2016-06-24 8:51 UTC (permalink / raw) To: Joe Perches Cc: Patrick McHardy, Jozsef Kadlecsik, Stephen Hemminger, David S. Miller, netfilter-devel, coreteam, bridge, netdev, linux-kernel On Thu, Jun 23, 2016 at 12:00:00PM -0700, Joe Perches wrote: > On Thu, 2016-06-23 at 19:36 +0200, Pablo Neira Ayuso wrote: > > On Wed, Jun 15, 2016 at 01:58:45PM -0700, Joe Perches wrote: > > > > > > There is code duplication of a masked ethernet address comparison here > > > so make it a separate function instead. > > > > > > Miscellanea: > > > > > > o Neaten alignment of FWINV macro uses to make it clearer for the reader > > Applied, thanks. > > > > > > > > Signed-off-by: Joe Perches <joe@perches.com> > > > --- > > > > > > This masked_ether_addr_equal function could go into etherdevice.h, > > > but I don't see another use like it in kernel code. Is there one? > > > > This is specific of iptables, not even nftables would use this. So I > > would keep this in the iptables tree. > > Did you see the other patch that adds a generic > ether_addr_equal_masked() and uses it in a few > more files? You mean this one: http://patchwork.ozlabs.org/patch/636208/ OK, so I'll toss the previous and will take this one instead. As I said my opinion is that ether_addr_equal_masked() is only required by netfilter, but thinking it well I don't really mind in what header this function is placed given that these are our internal headers. Thanks. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] bridge: netfilter: spanning tree: Add masked_ether_addr_equal and neatening 2016-06-24 8:51 ` Pablo Neira Ayuso (?) @ 2016-06-24 8:57 ` Pablo Neira Ayuso -1 siblings, 0 replies; 24+ messages in thread From: Pablo Neira Ayuso @ 2016-06-24 8:57 UTC (permalink / raw) To: Joe Perches Cc: Patrick McHardy, Jozsef Kadlecsik, Stephen Hemminger, David S. Miller, netfilter-devel, coreteam, bridge, netdev, linux-kernel On Fri, Jun 24, 2016 at 10:51:28AM +0200, Pablo Neira Ayuso wrote: > On Thu, Jun 23, 2016 at 12:00:00PM -0700, Joe Perches wrote: > > On Thu, 2016-06-23 at 19:36 +0200, Pablo Neira Ayuso wrote: > > > On Wed, Jun 15, 2016 at 01:58:45PM -0700, Joe Perches wrote: > > > > > > > > There is code duplication of a masked ethernet address comparison here > > > > so make it a separate function instead. > > > > > > > > Miscellanea: > > > > > > > > o Neaten alignment of FWINV macro uses to make it clearer for the reader > > > Applied, thanks. > > > > > > > > > > > Signed-off-by: Joe Perches <joe@perches.com> > > > > --- > > > > > > > > This masked_ether_addr_equal function could go into etherdevice.h, > > > > but I don't see another use like it in kernel code. Is there one? > > > > > > This is specific of iptables, not even nftables would use this. So I > > > would keep this in the iptables tree. > > > > Did you see the other patch that adds a generic > > ether_addr_equal_masked() and uses it in a few > > more files? > > You mean this one: > > http://patchwork.ozlabs.org/patch/636208/ > > OK, so I'll toss the previous and will take this one instead. > > As I said my opinion is that ether_addr_equal_masked() is only > required by netfilter, but thinking it well I don't really mind in > what header this function is placed given that these are our internal > headers. git am reports patch I get from patchwork is corrupt at line 37. Tried a couple of tricks to fix it but this didn't work. Would you mind resubmitting this patch? Sorry for the inconvenience. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Bridge] [PATCH] bridge: netfilter: spanning tree: Add masked_ether_addr_equal and neatening @ 2016-06-24 8:57 ` Pablo Neira Ayuso 0 siblings, 0 replies; 24+ messages in thread From: Pablo Neira Ayuso @ 2016-06-24 8:57 UTC (permalink / raw) To: Joe Perches Cc: netdev, bridge, linux-kernel, Patrick McHardy, coreteam, netfilter-devel, Jozsef Kadlecsik, David S. Miller On Fri, Jun 24, 2016 at 10:51:28AM +0200, Pablo Neira Ayuso wrote: > On Thu, Jun 23, 2016 at 12:00:00PM -0700, Joe Perches wrote: > > On Thu, 2016-06-23 at 19:36 +0200, Pablo Neira Ayuso wrote: > > > On Wed, Jun 15, 2016 at 01:58:45PM -0700, Joe Perches wrote: > > > > > > > > There is code duplication of a masked ethernet address comparison here > > > > so make it a separate function instead. > > > > > > > > Miscellanea: > > > > > > > > o Neaten alignment of FWINV macro uses to make it clearer for the reader > > > Applied, thanks. > > > > > > > > > > > Signed-off-by: Joe Perches <joe@perches.com> > > > > --- > > > > > > > > This masked_ether_addr_equal function could go into etherdevice.h, > > > > but I don't see another use like it in kernel code. Is there one? > > > > > > This is specific of iptables, not even nftables would use this. So I > > > would keep this in the iptables tree. > > > > Did you see the other patch that adds a generic > > ether_addr_equal_masked() and uses it in a few > > more files? > > You mean this one: > > http://patchwork.ozlabs.org/patch/636208/ > > OK, so I'll toss the previous and will take this one instead. > > As I said my opinion is that ether_addr_equal_masked() is only > required by netfilter, but thinking it well I don't really mind in > what header this function is placed given that these are our internal > headers. git am reports patch I get from patchwork is corrupt at line 37. Tried a couple of tricks to fix it but this didn't work. Would you mind resubmitting this patch? Sorry for the inconvenience. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] bridge: netfilter: spanning tree: Add masked_ether_addr_equal and neatening @ 2016-06-24 8:57 ` Pablo Neira Ayuso 0 siblings, 0 replies; 24+ messages in thread From: Pablo Neira Ayuso @ 2016-06-24 8:57 UTC (permalink / raw) To: Joe Perches Cc: Patrick McHardy, Jozsef Kadlecsik, Stephen Hemminger, David S. Miller, netfilter-devel, coreteam, bridge, netdev, linux-kernel On Fri, Jun 24, 2016 at 10:51:28AM +0200, Pablo Neira Ayuso wrote: > On Thu, Jun 23, 2016 at 12:00:00PM -0700, Joe Perches wrote: > > On Thu, 2016-06-23 at 19:36 +0200, Pablo Neira Ayuso wrote: > > > On Wed, Jun 15, 2016 at 01:58:45PM -0700, Joe Perches wrote: > > > > > > > > There is code duplication of a masked ethernet address comparison here > > > > so make it a separate function instead. > > > > > > > > Miscellanea: > > > > > > > > o Neaten alignment of FWINV macro uses to make it clearer for the reader > > > Applied, thanks. > > > > > > > > > > > Signed-off-by: Joe Perches <joe@perches.com> > > > > --- > > > > > > > > This masked_ether_addr_equal function could go into etherdevice.h, > > > > but I don't see another use like it in kernel code. Is there one? > > > > > > This is specific of iptables, not even nftables would use this. So I > > > would keep this in the iptables tree. > > > > Did you see the other patch that adds a generic > > ether_addr_equal_masked() and uses it in a few > > more files? > > You mean this one: > > http://patchwork.ozlabs.org/patch/636208/ > > OK, so I'll toss the previous and will take this one instead. > > As I said my opinion is that ether_addr_equal_masked() is only > required by netfilter, but thinking it well I don't really mind in > what header this function is placed given that these are our internal > headers. git am reports patch I get from patchwork is corrupt at line 37. Tried a couple of tricks to fix it but this didn't work. Would you mind resubmitting this patch? Sorry for the inconvenience. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH] etherdevice.h & bridge: netfilter: Add and use ether_addr_equal_masked 2016-06-24 8:57 ` Pablo Neira Ayuso @ 2016-06-24 18:32 ` Joe Perches -1 siblings, 0 replies; 24+ messages in thread From: Joe Perches @ 2016-06-24 18:32 UTC (permalink / raw) To: Pablo Neira Ayuso, Patrick McHardy, Jozsef Kadlecsik Cc: Stephen Hemminger, David S. Miller, netdev, linux-kernel, netfilter-devel, coreteam, bridge There are code duplications of a masked ethernet address comparison here so make it a separate function instead. Miscellanea: o Neaten alignment of FWINV macro uses to make it clearer for the reader Signed-off-by: Joe Perches <joe@perches.com> --- include/linux/etherdevice.h | 23 +++++++++++++++++++ net/bridge/netfilter/ebt_arp.c | 17 +++++--------- net/bridge/netfilter/ebt_stp.c | 49 ++++++++++++++++++----------------------- net/bridge/netfilter/ebtables.c | 17 +++++--------- 4 files changed, 57 insertions(+), 49 deletions(-) diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h index 37ff4a6..6fec9e8 100644 --- a/include/linux/etherdevice.h +++ b/include/linux/etherdevice.h @@ -374,6 +374,29 @@ static inline bool ether_addr_equal_unaligned(const u8 *addr1, const u8 *addr2) } /** + * ether_addr_equal_masked - Compare two Ethernet addresses with a mask + * @addr1: Pointer to a six-byte array containing the 1st Ethernet address + * @addr2: Pointer to a six-byte array containing the 2nd Ethernet address + * @mask: Pointer to a six-byte array containing the Ethernet address bitmask + * + * Compare two Ethernet addresses with a mask, returns true if for every bit + * set in the bitmask the equivalent bits in the ethernet addresses are equal. + * Using a mask with all bits set is a slower ether_addr_equal. + */ +static inline bool ether_addr_equal_masked(const u8 *addr1, const u8 *addr2, + const u8 *mask) +{ + int i; + + for (i = 0; i < ETH_ALEN; i++) { + if ((addr1[i] ^ addr2[i]) & mask[i]) + return false; + } + + return true; +} + +/** * is_etherdev_addr - Tell if given Ethernet address belongs to the device. * @dev: Pointer to a device structure * @addr: Pointer to a six-byte array containing the Ethernet address diff --git a/net/bridge/netfilter/ebt_arp.c b/net/bridge/netfilter/ebt_arp.c index cd457b8..cca0a89 100644 --- a/net/bridge/netfilter/ebt_arp.c +++ b/net/bridge/netfilter/ebt_arp.c @@ -65,7 +65,6 @@ ebt_arp_mt(const struct sk_buff *skb, struct xt_action_param *par) if (info->bitmask & (EBT_ARP_SRC_MAC | EBT_ARP_DST_MAC)) { const unsigned char *mp; unsigned char _mac[ETH_ALEN]; - uint8_t verdict, i; if (ah->ar_hln != ETH_ALEN || ah->ar_hrd != htons(ARPHRD_ETHER)) return false; @@ -74,11 +73,9 @@ ebt_arp_mt(const struct sk_buff *skb, struct xt_action_param *par) sizeof(_mac), &_mac); if (mp == NULL) return false; - verdict = 0; - for (i = 0; i < 6; i++) - verdict |= (mp[i] ^ info->smaddr[i]) & - info->smmsk[i]; - if (FWINV(verdict != 0, EBT_ARP_SRC_MAC)) + if (FWINV(!ether_addr_equal_masked(mp, info->smaddr, + info->smmsk), + EBT_ARP_SRC_MAC)) return false; } @@ -88,11 +85,9 @@ ebt_arp_mt(const struct sk_buff *skb, struct xt_action_param *par) sizeof(_mac), &_mac); if (mp == NULL) return false; - verdict = 0; - for (i = 0; i < 6; i++) - verdict |= (mp[i] ^ info->dmaddr[i]) & - info->dmmsk[i]; - if (FWINV(verdict != 0, EBT_ARP_DST_MAC)) + if (FWINV(!ether_addr_equal_masked(mp, info->dmaddr, + info->dmmsk), + EBT_ARP_DST_MAC)) return false; } } diff --git a/net/bridge/netfilter/ebt_stp.c b/net/bridge/netfilter/ebt_stp.c index e77f90b..45f73d5 100644 --- a/net/bridge/netfilter/ebt_stp.c +++ b/net/bridge/netfilter/ebt_stp.c @@ -46,7 +46,6 @@ static bool ebt_filter_config(const struct ebt_stp_info *info, const struct ebt_stp_config_info *c; u16 v16; u32 v32; - int verdict, i; c = &info->config; if ((info->bitmask & EBT_STP_FLAGS) && @@ -54,66 +53,62 @@ static bool ebt_filter_config(const struct ebt_stp_info *info, return false; if (info->bitmask & EBT_STP_ROOTPRIO) { v16 = NR16(stpc->root); - if (FWINV(v16 < c->root_priol || - v16 > c->root_priou, EBT_STP_ROOTPRIO)) + if (FWINV(v16 < c->root_priol || v16 > c->root_priou, + EBT_STP_ROOTPRIO)) return false; } if (info->bitmask & EBT_STP_ROOTADDR) { - verdict = 0; - for (i = 0; i < 6; i++) - verdict |= (stpc->root[2+i] ^ c->root_addr[i]) & - c->root_addrmsk[i]; - if (FWINV(verdict != 0, EBT_STP_ROOTADDR)) + if (FWINV(!ether_addr_equal_masked(&stpc->root[2], c->root_addr, + c->root_addrmsk), + EBT_STP_ROOTADDR)) return false; } if (info->bitmask & EBT_STP_ROOTCOST) { v32 = NR32(stpc->root_cost); - if (FWINV(v32 < c->root_costl || - v32 > c->root_costu, EBT_STP_ROOTCOST)) + if (FWINV(v32 < c->root_costl || v32 > c->root_costu, + EBT_STP_ROOTCOST)) return false; } if (info->bitmask & EBT_STP_SENDERPRIO) { v16 = NR16(stpc->sender); - if (FWINV(v16 < c->sender_priol || - v16 > c->sender_priou, EBT_STP_SENDERPRIO)) + if (FWINV(v16 < c->sender_priol || v16 > c->sender_priou, + EBT_STP_SENDERPRIO)) return false; } if (info->bitmask & EBT_STP_SENDERADDR) { - verdict = 0; - for (i = 0; i < 6; i++) - verdict |= (stpc->sender[2+i] ^ c->sender_addr[i]) & - c->sender_addrmsk[i]; - if (FWINV(verdict != 0, EBT_STP_SENDERADDR)) + if (FWINV(!ether_addr_equal_masked(&stpc->sender[2], + c->sender_addr, + c->sender_addrmsk), + EBT_STP_SENDERADDR)) return false; } if (info->bitmask & EBT_STP_PORT) { v16 = NR16(stpc->port); - if (FWINV(v16 < c->portl || - v16 > c->portu, EBT_STP_PORT)) + if (FWINV(v16 < c->portl || v16 > c->portu, EBT_STP_PORT)) return false; } if (info->bitmask & EBT_STP_MSGAGE) { v16 = NR16(stpc->msg_age); - if (FWINV(v16 < c->msg_agel || - v16 > c->msg_ageu, EBT_STP_MSGAGE)) + if (FWINV(v16 < c->msg_agel || v16 > c->msg_ageu, + EBT_STP_MSGAGE)) return false; } if (info->bitmask & EBT_STP_MAXAGE) { v16 = NR16(stpc->max_age); - if (FWINV(v16 < c->max_agel || - v16 > c->max_ageu, EBT_STP_MAXAGE)) + if (FWINV(v16 < c->max_agel || v16 > c->max_ageu, + EBT_STP_MAXAGE)) return false; } if (info->bitmask & EBT_STP_HELLOTIME) { v16 = NR16(stpc->hello_time); - if (FWINV(v16 < c->hello_timel || - v16 > c->hello_timeu, EBT_STP_HELLOTIME)) + if (FWINV(v16 < c->hello_timel || v16 > c->hello_timeu, + EBT_STP_HELLOTIME)) return false; } if (info->bitmask & EBT_STP_FWDD) { v16 = NR16(stpc->forward_delay); - if (FWINV(v16 < c->forward_delayl || - v16 > c->forward_delayu, EBT_STP_FWDD)) + if (FWINV(v16 < c->forward_delayl || v16 > c->forward_delayu, + EBT_STP_FWDD)) return false; } return true; diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c index 5a61f35..5721a25 100644 --- a/net/bridge/netfilter/ebtables.c +++ b/net/bridge/netfilter/ebtables.c @@ -130,7 +130,6 @@ ebt_basic_match(const struct ebt_entry *e, const struct sk_buff *skb, const struct ethhdr *h = eth_hdr(skb); const struct net_bridge_port *p; __be16 ethproto; - int verdict, i; if (skb_vlan_tag_present(skb)) ethproto = htons(ETH_P_8021Q); @@ -157,19 +156,15 @@ ebt_basic_match(const struct ebt_entry *e, const struct sk_buff *skb, return 1; if (e->bitmask & EBT_SOURCEMAC) { - verdict = 0; - for (i = 0; i < 6; i++) - verdict |= (h->h_source[i] ^ e->sourcemac[i]) & - e->sourcemsk[i]; - if (FWINV2(verdict != 0, EBT_ISOURCE)) + if (FWINV2(!ether_addr_equal_masked(h->h_source, + e->sourcemac, e->sourcemsk), + EBT_ISOURCE)) return 1; } if (e->bitmask & EBT_DESTMAC) { - verdict = 0; - for (i = 0; i < 6; i++) - verdict |= (h->h_dest[i] ^ e->destmac[i]) & - e->destmsk[i]; - if (FWINV2(verdict != 0, EBT_IDEST)) + if (FWINV2(!ether_addr_equal_masked(h->h_dest, + e->destmac, e->destmsk), + EBT_IDEST)) return 1; } return 0; -- 2.8.0.rc4.16.g56331f8 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Bridge] [PATCH] etherdevice.h & bridge: netfilter: Add and use ether_addr_equal_masked @ 2016-06-24 18:32 ` Joe Perches 0 siblings, 0 replies; 24+ messages in thread From: Joe Perches @ 2016-06-24 18:32 UTC (permalink / raw) To: Pablo Neira Ayuso, Patrick McHardy, Jozsef Kadlecsik Cc: netdev, bridge, linux-kernel, coreteam, netfilter-devel, David S. Miller There are code duplications of a masked ethernet address comparison here so make it a separate function instead. Miscellanea: o Neaten alignment of FWINV macro uses to make it clearer for the reader Signed-off-by: Joe Perches <joe@perches.com> --- include/linux/etherdevice.h | 23 +++++++++++++++++++ net/bridge/netfilter/ebt_arp.c | 17 +++++--------- net/bridge/netfilter/ebt_stp.c | 49 ++++++++++++++++++----------------------- net/bridge/netfilter/ebtables.c | 17 +++++--------- 4 files changed, 57 insertions(+), 49 deletions(-) diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h index 37ff4a6..6fec9e8 100644 --- a/include/linux/etherdevice.h +++ b/include/linux/etherdevice.h @@ -374,6 +374,29 @@ static inline bool ether_addr_equal_unaligned(const u8 *addr1, const u8 *addr2) } /** + * ether_addr_equal_masked - Compare two Ethernet addresses with a mask + * @addr1: Pointer to a six-byte array containing the 1st Ethernet address + * @addr2: Pointer to a six-byte array containing the 2nd Ethernet address + * @mask: Pointer to a six-byte array containing the Ethernet address bitmask + * + * Compare two Ethernet addresses with a mask, returns true if for every bit + * set in the bitmask the equivalent bits in the ethernet addresses are equal. + * Using a mask with all bits set is a slower ether_addr_equal. + */ +static inline bool ether_addr_equal_masked(const u8 *addr1, const u8 *addr2, + const u8 *mask) +{ + int i; + + for (i = 0; i < ETH_ALEN; i++) { + if ((addr1[i] ^ addr2[i]) & mask[i]) + return false; + } + + return true; +} + +/** * is_etherdev_addr - Tell if given Ethernet address belongs to the device. * @dev: Pointer to a device structure * @addr: Pointer to a six-byte array containing the Ethernet address diff --git a/net/bridge/netfilter/ebt_arp.c b/net/bridge/netfilter/ebt_arp.c index cd457b8..cca0a89 100644 --- a/net/bridge/netfilter/ebt_arp.c +++ b/net/bridge/netfilter/ebt_arp.c @@ -65,7 +65,6 @@ ebt_arp_mt(const struct sk_buff *skb, struct xt_action_param *par) if (info->bitmask & (EBT_ARP_SRC_MAC | EBT_ARP_DST_MAC)) { const unsigned char *mp; unsigned char _mac[ETH_ALEN]; - uint8_t verdict, i; if (ah->ar_hln != ETH_ALEN || ah->ar_hrd != htons(ARPHRD_ETHER)) return false; @@ -74,11 +73,9 @@ ebt_arp_mt(const struct sk_buff *skb, struct xt_action_param *par) sizeof(_mac), &_mac); if (mp == NULL) return false; - verdict = 0; - for (i = 0; i < 6; i++) - verdict |= (mp[i] ^ info->smaddr[i]) & - info->smmsk[i]; - if (FWINV(verdict != 0, EBT_ARP_SRC_MAC)) + if (FWINV(!ether_addr_equal_masked(mp, info->smaddr, + info->smmsk), + EBT_ARP_SRC_MAC)) return false; } @@ -88,11 +85,9 @@ ebt_arp_mt(const struct sk_buff *skb, struct xt_action_param *par) sizeof(_mac), &_mac); if (mp == NULL) return false; - verdict = 0; - for (i = 0; i < 6; i++) - verdict |= (mp[i] ^ info->dmaddr[i]) & - info->dmmsk[i]; - if (FWINV(verdict != 0, EBT_ARP_DST_MAC)) + if (FWINV(!ether_addr_equal_masked(mp, info->dmaddr, + info->dmmsk), + EBT_ARP_DST_MAC)) return false; } } diff --git a/net/bridge/netfilter/ebt_stp.c b/net/bridge/netfilter/ebt_stp.c index e77f90b..45f73d5 100644 --- a/net/bridge/netfilter/ebt_stp.c +++ b/net/bridge/netfilter/ebt_stp.c @@ -46,7 +46,6 @@ static bool ebt_filter_config(const struct ebt_stp_info *info, const struct ebt_stp_config_info *c; u16 v16; u32 v32; - int verdict, i; c = &info->config; if ((info->bitmask & EBT_STP_FLAGS) && @@ -54,66 +53,62 @@ static bool ebt_filter_config(const struct ebt_stp_info *info, return false; if (info->bitmask & EBT_STP_ROOTPRIO) { v16 = NR16(stpc->root); - if (FWINV(v16 < c->root_priol || - v16 > c->root_priou, EBT_STP_ROOTPRIO)) + if (FWINV(v16 < c->root_priol || v16 > c->root_priou, + EBT_STP_ROOTPRIO)) return false; } if (info->bitmask & EBT_STP_ROOTADDR) { - verdict = 0; - for (i = 0; i < 6; i++) - verdict |= (stpc->root[2+i] ^ c->root_addr[i]) & - c->root_addrmsk[i]; - if (FWINV(verdict != 0, EBT_STP_ROOTADDR)) + if (FWINV(!ether_addr_equal_masked(&stpc->root[2], c->root_addr, + c->root_addrmsk), + EBT_STP_ROOTADDR)) return false; } if (info->bitmask & EBT_STP_ROOTCOST) { v32 = NR32(stpc->root_cost); - if (FWINV(v32 < c->root_costl || - v32 > c->root_costu, EBT_STP_ROOTCOST)) + if (FWINV(v32 < c->root_costl || v32 > c->root_costu, + EBT_STP_ROOTCOST)) return false; } if (info->bitmask & EBT_STP_SENDERPRIO) { v16 = NR16(stpc->sender); - if (FWINV(v16 < c->sender_priol || - v16 > c->sender_priou, EBT_STP_SENDERPRIO)) + if (FWINV(v16 < c->sender_priol || v16 > c->sender_priou, + EBT_STP_SENDERPRIO)) return false; } if (info->bitmask & EBT_STP_SENDERADDR) { - verdict = 0; - for (i = 0; i < 6; i++) - verdict |= (stpc->sender[2+i] ^ c->sender_addr[i]) & - c->sender_addrmsk[i]; - if (FWINV(verdict != 0, EBT_STP_SENDERADDR)) + if (FWINV(!ether_addr_equal_masked(&stpc->sender[2], + c->sender_addr, + c->sender_addrmsk), + EBT_STP_SENDERADDR)) return false; } if (info->bitmask & EBT_STP_PORT) { v16 = NR16(stpc->port); - if (FWINV(v16 < c->portl || - v16 > c->portu, EBT_STP_PORT)) + if (FWINV(v16 < c->portl || v16 > c->portu, EBT_STP_PORT)) return false; } if (info->bitmask & EBT_STP_MSGAGE) { v16 = NR16(stpc->msg_age); - if (FWINV(v16 < c->msg_agel || - v16 > c->msg_ageu, EBT_STP_MSGAGE)) + if (FWINV(v16 < c->msg_agel || v16 > c->msg_ageu, + EBT_STP_MSGAGE)) return false; } if (info->bitmask & EBT_STP_MAXAGE) { v16 = NR16(stpc->max_age); - if (FWINV(v16 < c->max_agel || - v16 > c->max_ageu, EBT_STP_MAXAGE)) + if (FWINV(v16 < c->max_agel || v16 > c->max_ageu, + EBT_STP_MAXAGE)) return false; } if (info->bitmask & EBT_STP_HELLOTIME) { v16 = NR16(stpc->hello_time); - if (FWINV(v16 < c->hello_timel || - v16 > c->hello_timeu, EBT_STP_HELLOTIME)) + if (FWINV(v16 < c->hello_timel || v16 > c->hello_timeu, + EBT_STP_HELLOTIME)) return false; } if (info->bitmask & EBT_STP_FWDD) { v16 = NR16(stpc->forward_delay); - if (FWINV(v16 < c->forward_delayl || - v16 > c->forward_delayu, EBT_STP_FWDD)) + if (FWINV(v16 < c->forward_delayl || v16 > c->forward_delayu, + EBT_STP_FWDD)) return false; } return true; diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c index 5a61f35..5721a25 100644 --- a/net/bridge/netfilter/ebtables.c +++ b/net/bridge/netfilter/ebtables.c @@ -130,7 +130,6 @@ ebt_basic_match(const struct ebt_entry *e, const struct sk_buff *skb, const struct ethhdr *h = eth_hdr(skb); const struct net_bridge_port *p; __be16 ethproto; - int verdict, i; if (skb_vlan_tag_present(skb)) ethproto = htons(ETH_P_8021Q); @@ -157,19 +156,15 @@ ebt_basic_match(const struct ebt_entry *e, const struct sk_buff *skb, return 1; if (e->bitmask & EBT_SOURCEMAC) { - verdict = 0; - for (i = 0; i < 6; i++) - verdict |= (h->h_source[i] ^ e->sourcemac[i]) & - e->sourcemsk[i]; - if (FWINV2(verdict != 0, EBT_ISOURCE)) + if (FWINV2(!ether_addr_equal_masked(h->h_source, + e->sourcemac, e->sourcemsk), + EBT_ISOURCE)) return 1; } if (e->bitmask & EBT_DESTMAC) { - verdict = 0; - for (i = 0; i < 6; i++) - verdict |= (h->h_dest[i] ^ e->destmac[i]) & - e->destmsk[i]; - if (FWINV2(verdict != 0, EBT_IDEST)) + if (FWINV2(!ether_addr_equal_masked(h->h_dest, + e->destmac, e->destmsk), + EBT_IDEST)) return 1; } return 0; -- 2.8.0.rc4.16.g56331f8 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH] etherdevice.h & bridge: netfilter: Add and use ether_addr_equal_masked 2016-06-24 18:32 ` [Bridge] " Joe Perches (?) @ 2016-06-28 13:01 ` David Miller -1 siblings, 0 replies; 24+ messages in thread From: David Miller @ 2016-06-28 13:01 UTC (permalink / raw) To: joe Cc: pablo, kaber, kadlec, stephen, netdev, linux-kernel, netfilter-devel, coreteam, bridge From: Joe Perches <joe@perches.com> Date: Fri, 24 Jun 2016 11:32:26 -0700 > There are code duplications of a masked ethernet address comparison here > so make it a separate function instead. > > Miscellanea: > > o Neaten alignment of FWINV macro uses to make it clearer for the reader > > Signed-off-by: Joe Perches <joe@perches.com> Pablo feel free to take this: Acked-by: David S. Miller <davem@davemloft.net> ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Bridge] [PATCH] etherdevice.h & bridge: netfilter: Add and use ether_addr_equal_masked @ 2016-06-28 13:01 ` David Miller 0 siblings, 0 replies; 24+ messages in thread From: David Miller @ 2016-06-28 13:01 UTC (permalink / raw) To: joe Cc: netdev, bridge, linux-kernel, coreteam, netfilter-devel, kadlec, kaber, pablo From: Joe Perches <joe@perches.com> Date: Fri, 24 Jun 2016 11:32:26 -0700 > There are code duplications of a masked ethernet address comparison here > so make it a separate function instead. > > Miscellanea: > > o Neaten alignment of FWINV macro uses to make it clearer for the reader > > Signed-off-by: Joe Perches <joe@perches.com> Pablo feel free to take this: Acked-by: David S. Miller <davem@davemloft.net> ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] etherdevice.h & bridge: netfilter: Add and use ether_addr_equal_masked @ 2016-06-28 13:01 ` David Miller 0 siblings, 0 replies; 24+ messages in thread From: David Miller @ 2016-06-28 13:01 UTC (permalink / raw) To: joe Cc: netdev, bridge, linux-kernel, coreteam, netfilter-devel, kadlec, kaber, pablo From: Joe Perches <joe@perches.com> Date: Fri, 24 Jun 2016 11:32:26 -0700 > There are code duplications of a masked ethernet address comparison here > so make it a separate function instead. > > Miscellanea: > > o Neaten alignment of FWINV macro uses to make it clearer for the reader > > Signed-off-by: Joe Perches <joe@perches.com> Pablo feel free to take this: Acked-by: David S. Miller <davem@davemloft.net> ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] etherdevice.h & bridge: netfilter: Add and use ether_addr_equal_masked 2016-06-28 13:01 ` David Miller (?) @ 2016-06-30 9:26 ` Pablo Neira Ayuso -1 siblings, 0 replies; 24+ messages in thread From: Pablo Neira Ayuso @ 2016-06-30 9:26 UTC (permalink / raw) To: David Miller Cc: joe, kaber, kadlec, stephen, netdev, linux-kernel, netfilter-devel, coreteam, bridge On Tue, Jun 28, 2016 at 09:01:12AM -0400, David Miller wrote: > From: Joe Perches <joe@perches.com> > Date: Fri, 24 Jun 2016 11:32:26 -0700 > > > There are code duplications of a masked ethernet address comparison here > > so make it a separate function instead. > > > > Miscellanea: > > > > o Neaten alignment of FWINV macro uses to make it clearer for the reader > > > > Signed-off-by: Joe Perches <joe@perches.com> > > Pablo feel free to take this: > > Acked-by: David S. Miller <davem@davemloft.net> Applied, thanks! ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Bridge] [PATCH] etherdevice.h & bridge: netfilter: Add and use ether_addr_equal_masked @ 2016-06-30 9:26 ` Pablo Neira Ayuso 0 siblings, 0 replies; 24+ messages in thread From: Pablo Neira Ayuso @ 2016-06-30 9:26 UTC (permalink / raw) To: David Miller Cc: netdev, bridge, linux-kernel, coreteam, netfilter-devel, kadlec, joe, kaber On Tue, Jun 28, 2016 at 09:01:12AM -0400, David Miller wrote: > From: Joe Perches <joe@perches.com> > Date: Fri, 24 Jun 2016 11:32:26 -0700 > > > There are code duplications of a masked ethernet address comparison here > > so make it a separate function instead. > > > > Miscellanea: > > > > o Neaten alignment of FWINV macro uses to make it clearer for the reader > > > > Signed-off-by: Joe Perches <joe@perches.com> > > Pablo feel free to take this: > > Acked-by: David S. Miller <davem@davemloft.net> Applied, thanks! ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] etherdevice.h & bridge: netfilter: Add and use ether_addr_equal_masked @ 2016-06-30 9:26 ` Pablo Neira Ayuso 0 siblings, 0 replies; 24+ messages in thread From: Pablo Neira Ayuso @ 2016-06-30 9:26 UTC (permalink / raw) To: David Miller Cc: netdev, bridge, linux-kernel, coreteam, netfilter-devel, kadlec, joe, kaber On Tue, Jun 28, 2016 at 09:01:12AM -0400, David Miller wrote: > From: Joe Perches <joe@perches.com> > Date: Fri, 24 Jun 2016 11:32:26 -0700 > > > There are code duplications of a masked ethernet address comparison here > > so make it a separate function instead. > > > > Miscellanea: > > > > o Neaten alignment of FWINV macro uses to make it clearer for the reader > > > > Signed-off-by: Joe Perches <joe@perches.com> > > Pablo feel free to take this: > > Acked-by: David S. Miller <davem@davemloft.net> Applied, thanks! ^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2016-06-30 9:28 UTC | newest] Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-06-15 20:58 [PATCH] bridge: netfilter: spanning tree: Add masked_ether_addr_equal and neatening Joe Perches 2016-06-15 20:58 ` [Bridge] " Joe Perches 2016-06-16 6:04 ` Joe Perches 2016-06-16 6:04 ` [Bridge] " Joe Perches 2016-06-16 6:04 ` Joe Perches 2016-06-23 17:36 ` Pablo Neira Ayuso 2016-06-23 17:36 ` [Bridge] " Pablo Neira Ayuso 2016-06-23 17:36 ` Pablo Neira Ayuso 2016-06-23 19:00 ` Joe Perches 2016-06-23 19:00 ` [Bridge] " Joe Perches 2016-06-24 8:51 ` Pablo Neira Ayuso 2016-06-24 8:51 ` [Bridge] " Pablo Neira Ayuso 2016-06-24 8:51 ` Pablo Neira Ayuso 2016-06-24 8:57 ` Pablo Neira Ayuso 2016-06-24 8:57 ` [Bridge] " Pablo Neira Ayuso 2016-06-24 8:57 ` Pablo Neira Ayuso 2016-06-24 18:32 ` [PATCH] etherdevice.h & bridge: netfilter: Add and use ether_addr_equal_masked Joe Perches 2016-06-24 18:32 ` [Bridge] " Joe Perches 2016-06-28 13:01 ` David Miller 2016-06-28 13:01 ` [Bridge] " David Miller 2016-06-28 13:01 ` David Miller 2016-06-30 9:26 ` Pablo Neira Ayuso 2016-06-30 9:26 ` [Bridge] " Pablo Neira Ayuso 2016-06-30 9:26 ` Pablo Neira Ayuso
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.