* [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).