* [PATCH net-next] ethernet: rocker: Use is_broadcast_ether_addr() and is_multicast_ether_addr() instead of ether_addr_equal() @ 2023-08-14 2:29 Ruan Jinjie 2023-08-14 7:47 ` Simon Horman 0 siblings, 1 reply; 4+ messages in thread From: Ruan Jinjie @ 2023-08-14 2:29 UTC (permalink / raw) To: netdev, Jiri Pirko, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni Cc: ruanjinjie Use is_broadcast_ether_addr() and is_multicast_ether_addr() instead of ether_addr_equal() to check if the ethernet address is broadcast and multicast address separately. Signed-off-by: Ruan Jinjie <ruanjinjie@huawei.com> --- drivers/net/ethernet/rocker/rocker_ofdpa.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/rocker/rocker_ofdpa.c b/drivers/net/ethernet/rocker/rocker_ofdpa.c index 826990459fa4..7f389f3adbf4 100644 --- a/drivers/net/ethernet/rocker/rocker_ofdpa.c +++ b/drivers/net/ethernet/rocker/rocker_ofdpa.c @@ -208,7 +208,6 @@ static const u8 zero_mac[ETH_ALEN] = { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 }; static const u8 ff_mac[ETH_ALEN] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff }; static const u8 ll_mac[ETH_ALEN] = { 0x01, 0x80, 0xc2, 0x00, 0x00, 0x00 }; static const u8 ll_mask[ETH_ALEN] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xf0 }; -static const u8 mcast_mac[ETH_ALEN] = { 0x01, 0x00, 0x00, 0x00, 0x00, 0x00 }; static const u8 ipv4_mcast[ETH_ALEN] = { 0x01, 0x00, 0x5e, 0x00, 0x00, 0x00 }; static const u8 ipv4_mask[ETH_ALEN] = { 0xff, 0xff, 0xff, 0x80, 0x00, 0x00 }; static const u8 ipv6_mcast[ETH_ALEN] = { 0x33, 0x33, 0x00, 0x00, 0x00, 0x00 }; @@ -939,7 +938,7 @@ static int ofdpa_flow_tbl_bridge(struct ofdpa_port *ofdpa_port, if (eth_dst_mask) { entry->key.bridge.has_eth_dst_mask = 1; ether_addr_copy(entry->key.bridge.eth_dst_mask, eth_dst_mask); - if (!ether_addr_equal(eth_dst_mask, ff_mac)) + if (!is_broadcast_ether_addr(eth_dst_mask)) wild = true; } @@ -1012,7 +1011,7 @@ static int ofdpa_flow_tbl_acl(struct ofdpa_port *ofdpa_port, int flags, priority = OFDPA_PRIORITY_ACL_NORMAL; if (eth_dst && eth_dst_mask) { - if (ether_addr_equal(eth_dst_mask, mcast_mac)) + if (is_multicast_ether_addr(eth_dst_mask)) priority = OFDPA_PRIORITY_ACL_DFLT; else if (is_link_local_ether_addr(eth_dst)) priority = OFDPA_PRIORITY_ACL_CTRL; -- 2.34.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH net-next] ethernet: rocker: Use is_broadcast_ether_addr() and is_multicast_ether_addr() instead of ether_addr_equal() 2023-08-14 2:29 [PATCH net-next] ethernet: rocker: Use is_broadcast_ether_addr() and is_multicast_ether_addr() instead of ether_addr_equal() Ruan Jinjie @ 2023-08-14 7:47 ` Simon Horman 2023-08-14 8:32 ` Ruan Jinjie 0 siblings, 1 reply; 4+ messages in thread From: Simon Horman @ 2023-08-14 7:47 UTC (permalink / raw) To: Ruan Jinjie Cc: netdev, Jiri Pirko, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni On Mon, Aug 14, 2023 at 10:29:48AM +0800, Ruan Jinjie wrote: > Use is_broadcast_ether_addr() and is_multicast_ether_addr() instead of > ether_addr_equal() to check if the ethernet address is broadcast > and multicast address separately. > > Signed-off-by: Ruan Jinjie <ruanjinjie@huawei.com> Hi, Perhaps we could go for a more concise prefix and subject, as the current one is rather long. Maybe something like: Subject: [PATCH net-next]: rocker: Use helpers to check broadcast and multicast Ether addresses > --- > drivers/net/ethernet/rocker/rocker_ofdpa.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/ethernet/rocker/rocker_ofdpa.c b/drivers/net/ethernet/rocker/rocker_ofdpa.c > index 826990459fa4..7f389f3adbf4 100644 > --- a/drivers/net/ethernet/rocker/rocker_ofdpa.c > +++ b/drivers/net/ethernet/rocker/rocker_ofdpa.c > @@ -208,7 +208,6 @@ static const u8 zero_mac[ETH_ALEN] = { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 }; > static const u8 ff_mac[ETH_ALEN] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff }; > static const u8 ll_mac[ETH_ALEN] = { 0x01, 0x80, 0xc2, 0x00, 0x00, 0x00 }; > static const u8 ll_mask[ETH_ALEN] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xf0 }; > -static const u8 mcast_mac[ETH_ALEN] = { 0x01, 0x00, 0x00, 0x00, 0x00, 0x00 }; > static const u8 ipv4_mcast[ETH_ALEN] = { 0x01, 0x00, 0x5e, 0x00, 0x00, 0x00 }; > static const u8 ipv4_mask[ETH_ALEN] = { 0xff, 0xff, 0xff, 0x80, 0x00, 0x00 }; > static const u8 ipv6_mcast[ETH_ALEN] = { 0x33, 0x33, 0x00, 0x00, 0x00, 0x00 }; > @@ -939,7 +938,7 @@ static int ofdpa_flow_tbl_bridge(struct ofdpa_port *ofdpa_port, > if (eth_dst_mask) { > entry->key.bridge.has_eth_dst_mask = 1; > ether_addr_copy(entry->key.bridge.eth_dst_mask, eth_dst_mask); > - if (!ether_addr_equal(eth_dst_mask, ff_mac)) > + if (!is_broadcast_ether_addr(eth_dst_mask)) Probably it is ok, but is_broadcast_ether_addr() covers a set of addresses that includes ff_mac. > wild = true; > } > > @@ -1012,7 +1011,7 @@ static int ofdpa_flow_tbl_acl(struct ofdpa_port *ofdpa_port, int flags, > > priority = OFDPA_PRIORITY_ACL_NORMAL; > if (eth_dst && eth_dst_mask) { > - if (ether_addr_equal(eth_dst_mask, mcast_mac)) > + if (is_multicast_ether_addr(eth_dst_mask)) Likewise, is_multicast_ether_addr() covers a set of addresses that includes mcast_mac. > priority = OFDPA_PRIORITY_ACL_DFLT; > else if (is_link_local_ether_addr(eth_dst)) > priority = OFDPA_PRIORITY_ACL_CTRL; > -- > 2.34.1 > > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net-next] ethernet: rocker: Use is_broadcast_ether_addr() and is_multicast_ether_addr() instead of ether_addr_equal() 2023-08-14 7:47 ` Simon Horman @ 2023-08-14 8:32 ` Ruan Jinjie 2023-08-14 15:13 ` Simon Horman 0 siblings, 1 reply; 4+ messages in thread From: Ruan Jinjie @ 2023-08-14 8:32 UTC (permalink / raw) To: Simon Horman Cc: netdev, Jiri Pirko, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni On 2023/8/14 15:47, Simon Horman wrote: > On Mon, Aug 14, 2023 at 10:29:48AM +0800, Ruan Jinjie wrote: >> Use is_broadcast_ether_addr() and is_multicast_ether_addr() instead of >> ether_addr_equal() to check if the ethernet address is broadcast >> and multicast address separately. >> >> Signed-off-by: Ruan Jinjie <ruanjinjie@huawei.com> > > Hi, > > Perhaps we could go for a more concise prefix and subject, as the current one > is rather long. Maybe something like: > > Subject: [PATCH net-next]: rocker: Use helpers to check broadcast and multicast Ether addresses Right！That is more concise. > >> --- >> drivers/net/ethernet/rocker/rocker_ofdpa.c | 5 ++--- >> 1 file changed, 2 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/net/ethernet/rocker/rocker_ofdpa.c b/drivers/net/ethernet/rocker/rocker_ofdpa.c >> index 826990459fa4..7f389f3adbf4 100644 >> --- a/drivers/net/ethernet/rocker/rocker_ofdpa.c >> +++ b/drivers/net/ethernet/rocker/rocker_ofdpa.c >> @@ -208,7 +208,6 @@ static const u8 zero_mac[ETH_ALEN] = { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 }; >> static const u8 ff_mac[ETH_ALEN] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff }; >> static const u8 ll_mac[ETH_ALEN] = { 0x01, 0x80, 0xc2, 0x00, 0x00, 0x00 }; >> static const u8 ll_mask[ETH_ALEN] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xf0 }; >> -static const u8 mcast_mac[ETH_ALEN] = { 0x01, 0x00, 0x00, 0x00, 0x00, 0x00 }; >> static const u8 ipv4_mcast[ETH_ALEN] = { 0x01, 0x00, 0x5e, 0x00, 0x00, 0x00 }; >> static const u8 ipv4_mask[ETH_ALEN] = { 0xff, 0xff, 0xff, 0x80, 0x00, 0x00 }; >> static const u8 ipv6_mcast[ETH_ALEN] = { 0x33, 0x33, 0x00, 0x00, 0x00, 0x00 }; >> @@ -939,7 +938,7 @@ static int ofdpa_flow_tbl_bridge(struct ofdpa_port *ofdpa_port, >> if (eth_dst_mask) { >> entry->key.bridge.has_eth_dst_mask = 1; >> ether_addr_copy(entry->key.bridge.eth_dst_mask, eth_dst_mask); >> - if (!ether_addr_equal(eth_dst_mask, ff_mac)) >> + if (!is_broadcast_ether_addr(eth_dst_mask)) > > Probably it is ok, but is_broadcast_ether_addr() > covers a set of addresses that includes ff_mac. I reconfirmed that they are equivalent, is_broadcast_ether_addr() requires all six bytes to be F. > >> wild = true; >> } >> >> @@ -1012,7 +1011,7 @@ static int ofdpa_flow_tbl_acl(struct ofdpa_port *ofdpa_port, int flags, >> >> priority = OFDPA_PRIORITY_ACL_NORMAL; >> if (eth_dst && eth_dst_mask) { >> - if (ether_addr_equal(eth_dst_mask, mcast_mac)) >> + if (is_multicast_ether_addr(eth_dst_mask)) > > Likewise, is_multicast_ether_addr() > covers a set of addresses that includes mcast_mac. They are not exactly equivalent，the address what the mcast_mac get is the subset of is_multicast_ether_addr(). > >> priority = OFDPA_PRIORITY_ACL_DFLT; >> else if (is_link_local_ether_addr(eth_dst)) >> priority = OFDPA_PRIORITY_ACL_CTRL; >> -- >> 2.34.1 >> >> ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net-next] ethernet: rocker: Use is_broadcast_ether_addr() and is_multicast_ether_addr() instead of ether_addr_equal() 2023-08-14 8:32 ` Ruan Jinjie @ 2023-08-14 15:13 ` Simon Horman 0 siblings, 0 replies; 4+ messages in thread From: Simon Horman @ 2023-08-14 15:13 UTC (permalink / raw) To: Ruan Jinjie Cc: Simon Horman, netdev, Jiri Pirko, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni On Mon, Aug 14, 2023 at 04:32:56PM +0800, Ruan Jinjie wrote: > > > On 2023/8/14 15:47, Simon Horman wrote: > > On Mon, Aug 14, 2023 at 10:29:48AM +0800, Ruan Jinjie wrote: > >> Use is_broadcast_ether_addr() and is_multicast_ether_addr() instead of > >> ether_addr_equal() to check if the ethernet address is broadcast > >> and multicast address separately. > >> > >> Signed-off-by: Ruan Jinjie <ruanjinjie@huawei.com> > > > > Hi, > > > > Perhaps we could go for a more concise prefix and subject, as the current one > > is rather long. Maybe something like: > > > > Subject: [PATCH net-next]: rocker: Use helpers to check broadcast and multicast Ether addresses > > Right！That is more concise. > > > > >> --- > >> drivers/net/ethernet/rocker/rocker_ofdpa.c | 5 ++--- > >> 1 file changed, 2 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/net/ethernet/rocker/rocker_ofdpa.c b/drivers/net/ethernet/rocker/rocker_ofdpa.c > >> index 826990459fa4..7f389f3adbf4 100644 > >> --- a/drivers/net/ethernet/rocker/rocker_ofdpa.c > >> +++ b/drivers/net/ethernet/rocker/rocker_ofdpa.c > >> @@ -208,7 +208,6 @@ static const u8 zero_mac[ETH_ALEN] = { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 }; > >> static const u8 ff_mac[ETH_ALEN] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff }; > >> static const u8 ll_mac[ETH_ALEN] = { 0x01, 0x80, 0xc2, 0x00, 0x00, 0x00 }; > >> static const u8 ll_mask[ETH_ALEN] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xf0 }; > >> -static const u8 mcast_mac[ETH_ALEN] = { 0x01, 0x00, 0x00, 0x00, 0x00, 0x00 }; > >> static const u8 ipv4_mcast[ETH_ALEN] = { 0x01, 0x00, 0x5e, 0x00, 0x00, 0x00 }; > >> static const u8 ipv4_mask[ETH_ALEN] = { 0xff, 0xff, 0xff, 0x80, 0x00, 0x00 }; > >> static const u8 ipv6_mcast[ETH_ALEN] = { 0x33, 0x33, 0x00, 0x00, 0x00, 0x00 }; > >> @@ -939,7 +938,7 @@ static int ofdpa_flow_tbl_bridge(struct ofdpa_port *ofdpa_port, > >> if (eth_dst_mask) { > >> entry->key.bridge.has_eth_dst_mask = 1; > >> ether_addr_copy(entry->key.bridge.eth_dst_mask, eth_dst_mask); > >> - if (!ether_addr_equal(eth_dst_mask, ff_mac)) > >> + if (!is_broadcast_ether_addr(eth_dst_mask)) > > > > Probably it is ok, but is_broadcast_ether_addr() > > covers a set of addresses that includes ff_mac. > > I reconfirmed that they are equivalent, is_broadcast_ether_addr() > requires all six bytes to be F. Right, agreed. Sorry for misunderstanding the implementation of is_broadcast_ether_addr() when I checked earlier today. > > > > >> wild = true; > >> } > >> > >> @@ -1012,7 +1011,7 @@ static int ofdpa_flow_tbl_acl(struct ofdpa_port *ofdpa_port, int flags, > >> > >> priority = OFDPA_PRIORITY_ACL_NORMAL; > >> if (eth_dst && eth_dst_mask) { > >> - if (ether_addr_equal(eth_dst_mask, mcast_mac)) > >> + if (is_multicast_ether_addr(eth_dst_mask)) > > > > Likewise, is_multicast_ether_addr() > > covers a set of addresses that includes mcast_mac. > > They are not exactly equivalent，the address what the mcast_mac get is > the subset of is_multicast_ether_addr(). > > > > >> priority = OFDPA_PRIORITY_ACL_DFLT; > >> else if (is_link_local_ether_addr(eth_dst)) > >> priority = OFDPA_PRIORITY_ACL_CTRL; > >> -- > >> 2.34.1 > >> > >> > ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-08-14 15:13 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-08-14 2:29 [PATCH net-next] ethernet: rocker: Use is_broadcast_ether_addr() and is_multicast_ether_addr() instead of ether_addr_equal() Ruan Jinjie 2023-08-14 7:47 ` Simon Horman 2023-08-14 8:32 ` Ruan Jinjie 2023-08-14 15:13 ` Simon Horman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).