net: Support Wake-on-LAN using filters

PATCH In review Florian Fainelli 3 participants 35 messages Jul 17, 2018 – Aug 3, 2018

Subsystems (6)

Networking Network drivers Networking DSA Networking ethtool +2 more subsystems

lore

Share & tools

lore

Florian Fainelli

author

2018-07-17 16:10 UTC #0 lore raw

cover net: Support Wake-on-LAN using filters

Hi all,

This patch series adds support for allowing Wake-on-LAN to wake-up the system using configurable filters. This is particular useful in the context of Android where wake on MDNS is a requirement.

We support this by using the bcm_sf2 Compact Field Processor (CFP) which supports matching packets and tagging them with an unique identifier (Classification ID) that is added in each packet being matched through the use of Broadcom tags. The SYSTEMPORT MAC attached to that switch is then used to match that unique identifier and trigger a system wake-up event.

Last patch is the ethtool modifications to support that feature.

Example:

ethtool --config-nfc gphy flow-type udp4 src-ip 192.168.1.1 dst-ip 192.168.1.32 \
	src-port 1234 dst-port 5678 action 64

Added rule with ID 1

ethtool -s gphy wol f filters 0x2

To wake up the device:

nc -vz -u -p 1234 -s 192.168.1.1 192.168.1.32 5678

Florian Fainelli (7):
net: dsa: bcm_sf2: Allow targeting CPU ports for CFP rules
net: dsa: bcm_sf2: Disable learning while in WoL
net: systemport: Do not re-configure upon WoL interrupt
net: systemport: Create helper to set MPD
ethtool: Add WAKE_FILTER bitmask
net: systemport: Add support for WAKE_FILTER
net: dsa: bcm_sf2: Support WAKE_FILTER

 drivers/net/dsa/bcm_sf2.c                  |  14 +++-
 drivers/net/dsa/bcm_sf2_cfp.c              |   3 +-
 drivers/net/dsa/bcm_sf2_regs.h             |   2 +
 drivers/net/ethernet/broadcom/bcmsysport.c | 129 ++++++++++++++++++++++++-----
 drivers/net/ethernet/broadcom/bcmsysport.h |  14 +++-
 include/uapi/linux/ethtool.h               |   3 +-
 6 files changed, 138 insertions(+), 27 deletions(-)

Signature · 1 line

2.14.1

Florian Fainelli

author

2018-07-17 16:10 UTC #1 lore raw

PATCH 1/1 ethtool: Add support for WAKE_FILTER

Allow re-purposing the wol->sopass storage area to specify a bitmask of filters (programmed previously via ethtool::rxnfc) to be used as wake-up patterns.

Signed-off-by Florian Fainelli <[email protected]>

---
 ethtool-copy.h |  1 +
 ethtool.c      | 35 ++++++++++++++++++++++++++++++++++-
 2 files changed, 35 insertions(+), 1 deletion(-)

ethtool-copy.h +1 -0

index 8cc61e9ab40b..dbfaca15dca5 100644
@@ -1628,6 +1628,7 @@ static __inline__ int ethtool_validate_duplex(__u8 duplex)
 #define WAKE_ARP		(1 << 4)
 #define WAKE_MAGIC		(1 << 5)
 #define WAKE_MAGICSECURE	(1 << 6) /* only meaningful if WAKE_MAGIC */
+#define WAKE_FILTER		(1 << 7)
 
 /* L2-L4 network traffic flow types */
 #define	TCP_V4_FLOW	0x01	/* hash or spec (tcp_ip4_spec) */

ethtool.c +34 -1

index fb93ae898312..322fc8d98ee5 100644
@@ -931,6 +931,9 @@ static int parse_wolopts(char *optstr, u32 *data)
 		case 's':
 			*data |= WAKE_MAGICSECURE;
 			break;
+		case 'f':
+			*data |= WAKE_FILTER;
+			break;
 		case 'd':
 			*data = 0;
 			break;

@@ -964,6 +967,8 @@ static char *unparse_wolopts(int wolopts)
 			*p++ = 'g';
 		if (wolopts & WAKE_MAGICSECURE)
 			*p++ = 's';
+		if (wolopts & WAKE_FILTER)
+			*p++ = 'f';
 	} else {
 		*p = 'd';
 	}

@@ -989,6 +994,21 @@ static int dump_wol(struct ethtool_wolinfo *wol)
 		fprintf(stdout, "\n");
 	}
 
+	if (wol->supported & WAKE_FILTER) {
+		int i, j;
+		int delim = 0;
+		fprintf(stdout, "        Filter(s) enabled: ");
+		for (i = 0; i < SOPASS_MAX; i++) {
+			for (j = 0; j < 8; j++) {
+				if (wol->sopass[i] & (1 << j)) {
+					fprintf(stdout, "%s%d", delim?",":"", i * 8 + j);
+					delim=1;
+				}
+			}
+		}
+		fprintf(stdout, "\n");
+	}
+
 	return 0;
 }

@@ -2897,6 +2917,16 @@ static int do_sset(struct cmd_context *ctx)
 				exit_bad_args();
 			get_mac_addr(argp[i], sopass_wanted);
 			sopass_change = 1;
+		} else if (!strcmp(argp[i], "filters")) {
+			gwol_changed = 1;
+			i++;
+			if (i >= argc)
+				exit_bad_args();
+			if (parse_hex_u32_bitmap(argp[i],
+						 SOPASS_MAX * 8,
+						 (unsigned int *)sopass_wanted))
+				exit_bad_args();
+			sopass_change = 1;
 		} else if (!strcmp(argp[i], "msglvl")) {
 			i++;
 			if (i >= argc)

@@ -3112,8 +3142,10 @@ static int do_sset(struct cmd_context *ctx)
 		if (err < 0) {
 			if (wol_change)
 				fprintf(stderr, "  not setting wol\n");
-			if (sopass_change)
+			if (sopass_change & wol.wolopts & WAKE_MAGICSECURE)
 				fprintf(stderr, "  not setting sopass\n");
+			if (sopass_change & wol.wolopts & WAKE_FILTER)
+				fprintf(stderr, "  not setting filters\n");
 		}
 	}

@@ -5066,6 +5098,7 @@ static const struct option {
 	  "		[ xcvr internal|external ]\n"
 	  "		[ wol p|u|m|b|a|g|s|d... ]\n"
 	  "		[ sopass %x:%x:%x:%x:%x:%x ]\n"
+	  "		[ filters %x ]\n"
 	  "		[ msglvl %d | msglvl type on|off ... ]\n" },
 	{ "-a|--show-pause", 1, do_gpause, "Show pause options" },
 	{ "-A|--pause", 1, do_spause, "Set pause options",

Signature · 1 line

2.14.1

Florian Fainelli

author

2018-07-31 00:03 UTC #2 lore raw

Re: [PATCH ethtool] ethtool: Add support for WAKE_FILTER

Quoted 2 lines · Florian Fainelli

Allow re-purposing the wol->sopass storage area to specify a bitmask of filters (programmed previously via ethtool::rxnfc) to be used as wake-up patterns.

John, David, can you provide some feedback if the approach is acceptable? I will address Andrew's comment about the user friendliness and allow providing a comma separate list of filter identifiers.

One usability issue with this approach is that one cannot specify wake-on-LAN using WAKE_MAGICSECURE *and* WAKE_FILTER at the same time, since it uses the same location in the ioctl() structure that is being passed. Do you see this as a problem?

Thanks!

Quoted 102 lines

Signed-off-by Florian Fainelli <[email protected]>

---
 ethtool-copy.h |  1 +
 ethtool.c      | 35 ++++++++++++++++++++++++++++++++++-
 2 files changed, 35 insertions(+), 1 deletion(-)

ethtool-copy.h +1 -0

index 8cc61e9ab40b..dbfaca15dca5 100644
@@ -1628,6 +1628,7 @@ static __inline__ int ethtool_validate_duplex(__u8 duplex)
 #define WAKE_ARP		(1 << 4)
 #define WAKE_MAGIC		(1 << 5)
 #define WAKE_MAGICSECURE	(1 << 6) /* only meaningful if WAKE_MAGIC */
+#define WAKE_FILTER		(1 << 7)
 
 /* L2-L4 network traffic flow types */
 #define	TCP_V4_FLOW	0x01	/* hash or spec (tcp_ip4_spec) */

ethtool.c +34 -1

index fb93ae898312..322fc8d98ee5 100644
@@ -931,6 +931,9 @@ static int parse_wolopts(char *optstr, u32 *data)
 		case 's':
 			*data |= WAKE_MAGICSECURE;
 			break;
+		case 'f':
+			*data |= WAKE_FILTER;
+			break;
 		case 'd':
 			*data = 0;
 			break;

@@ -964,6 +967,8 @@ static char *unparse_wolopts(int wolopts)
 			*p++ = 'g';
 		if (wolopts & WAKE_MAGICSECURE)
 			*p++ = 's';
+		if (wolopts & WAKE_FILTER)
+			*p++ = 'f';
 	} else {
 		*p = 'd';
 	}

@@ -989,6 +994,21 @@ static int dump_wol(struct ethtool_wolinfo *wol)
 		fprintf(stdout, "\n");
 	}
 
+	if (wol->supported & WAKE_FILTER) {
+		int i, j;
+		int delim = 0;
+		fprintf(stdout, "        Filter(s) enabled: ");
+		for (i = 0; i < SOPASS_MAX; i++) {
+			for (j = 0; j < 8; j++) {
+				if (wol->sopass[i] & (1 << j)) {
+					fprintf(stdout, "%s%d", delim?",":"", i * 8 + j);
+					delim=1;
+				}
+			}
+		}
+		fprintf(stdout, "\n");
+	}
+
 	return 0;
 }

@@ -2897,6 +2917,16 @@ static int do_sset(struct cmd_context *ctx)
 				exit_bad_args();
 			get_mac_addr(argp[i], sopass_wanted);
 			sopass_change = 1;
+		} else if (!strcmp(argp[i], "filters")) {
+			gwol_changed = 1;
+			i++;
+			if (i >= argc)
+				exit_bad_args();
+			if (parse_hex_u32_bitmap(argp[i],
+						 SOPASS_MAX * 8,
+						 (unsigned int *)sopass_wanted))
+				exit_bad_args();
+			sopass_change = 1;
 		} else if (!strcmp(argp[i], "msglvl")) {
 			i++;
 			if (i >= argc)

@@ -3112,8 +3142,10 @@ static int do_sset(struct cmd_context *ctx)
 		if (err < 0) {
 			if (wol_change)
 				fprintf(stderr, "  not setting wol\n");
-			if (sopass_change)
+			if (sopass_change & wol.wolopts & WAKE_MAGICSECURE)
 				fprintf(stderr, "  not setting sopass\n");
+			if (sopass_change & wol.wolopts & WAKE_FILTER)
+				fprintf(stderr, "  not setting filters\n");
 		}
 	}

@@ -5066,6 +5098,7 @@ static const struct option {
 	  "		[ xcvr internal|external ]\n"
 	  "		[ wol p|u|m|b|a|g|s|d... ]\n"
 	  "		[ sopass %x:%x:%x:%x:%x:%x ]\n"
+	  "		[ filters %x ]\n"
 	  "		[ msglvl %d | msglvl type on|off ... ]\n" },
 	{ "-a|--show-pause", 1, do_gpause, "Show pause options" },
 	{ "-A|--pause", 1, do_spause, "Set pause options",

Signature · 1 line

Florian

Andrew Lunn

maintainer

2018-07-31 00:07 UTC #3 lore raw

Re: [PATCH ethtool] ethtool: Add support for WAKE_FILTER

Quoted 4 lines

One usability issue with this approach is that one cannot specify wake-on-LAN using WAKE_MAGICSECURE *and* WAKE_FILTER at the same time, since it uses the same location in the ioctl() structure that is being passed. Do you see this as a problem?

Hi Florian

Maybe you can think ahead to when ethtool gets a netlink implementation, and you have more flexibility to pass both as attributes? How does this affect the in kernel API?

Andrew

Florian Fainelli

author

2018-07-31 00:16 UTC #4 lore raw

Re: [PATCH ethtool] ethtool: Add support for WAKE_FILTER

Quoted 10 lines · Andrew Lunn

Quoted 4 lines
One usability issue with this approach is that one cannot specify wake-on-LAN using WAKE_MAGICSECURE *and* WAKE_FILTER at the same time, since it uses the same location in the ioctl() structure that is being passed. Do you see this as a problem?
Hi Florian
Maybe you can think ahead to when ethtool gets a netlink implementation, and you have more flexibility to pass both as attributes? How does this affect the in kernel API?

The thing is that I need this now, but when Michal's work on ethtool being migrated to netlink settles, we should have have any issues adding a proper storage area for specifying filters anymore. The issue here is of course that the size and layout of ethtool_wolinfo is largely fixed and set in stone, and therefore inflexible.

Signature · 1 line

Florian

Andrew Lunn

maintainer

2018-07-31 00:32 UTC #5 lore raw

Re: [PATCH ethtool] ethtool: Add support for WAKE_FILTER

Quoted 5 lines

The thing is that I need this now, but when Michal's work on ethtool being migrated to netlink settles, we should have have any issues adding a proper storage area for specifying filters anymore. The issue here is of course that the size and layout of ethtool_wolinfo is largely fixed and set in stone, and therefore inflexible.

The version in uapi/linux/ethtool.h is fixed. But i think in order to implement this properly, you are going to have to change the internal structure passed to ethtool_ops->set_wol/get_wol, with the filter separated out from the sopass. ethtool_set_wol()/ethtool_get_wol() would then need to know how to correctly unpack the structure passed in the ioctl, where as the netlink socket code and just put attributes into structure members.

Andrew

Andrew Lunn

maintainer

2018-07-31 00:38 UTC #6 lore raw

Re: [PATCH ethtool] ethtool: Add support for WAKE_FILTER

In reply to Florian Fainelli

Quoted 4 lines

One usability issue with this approach is that one cannot specify wake-on-LAN using WAKE_MAGICSECURE *and* WAKE_FILTER at the same time, since it uses the same location in the ioctl() structure that is being passed. Do you see this as a problem?

Hi Florian

I think you missed adding a check for this. ethtool_set_wol() should check if both WAKE_FILTER and WAKE_MAGICSECURE are set and return EINVAL.

Andrew

David Miller

maintainer

2018-08-01 18:18 UTC #7 lore raw

Re: [PATCH ethtool] ethtool: Add support for WAKE_FILTER

In reply to Florian Fainelli

From: Florian Fainelli <[email protected]>
Date: Mon, 30 Jul 2018 15:26:24 -0700

Quoted 12 lines · Florian Fainelli

Quoted 2 lines
Allow re-purposing the wol->sopass storage area to specify a bitmask of filters (programmed previously via ethtool::rxnfc) to be used as wake-up patterns.
John, David, can you provide some feedback if the approach is acceptable? I will address Andrew's comment about the user friendliness and allow providing a comma separate list of filter identifiers.
One usability issue with this approach is that one cannot specify wake-on-LAN using WAKE_MAGICSECURE *and* WAKE_FILTER at the same time, since it uses the same location in the ioctl() structure that is being passed. Do you see this as a problem?

Once again we are stuck in this weird situation, a sort of limbo.

On the one hand, I don't want to block your work on the ethtool netlink stuff being done.

However it is clear that by using netlink attributes, it would be so much cleaner.

I honestly don't know what to say at this time. I wish I had a clear piece of advice and a way for everyone to move forward, and usually I do, but this time I really don't :-/

Florian Fainelli

author

2018-08-03 19:54 UTC #8 lore raw

Re: [PATCH ethtool] ethtool: Add support for WAKE_FILTER

Quoted 21 lines · David Miller

From: Florian Fainelli <[email protected]>
Date: Mon, 30 Jul 2018 15:26:24 -0700
Quoted 12 lines · Florian Fainelli
Quoted 2 lines
Allow re-purposing the wol->sopass storage area to specify a bitmask of filters (programmed previously via ethtool::rxnfc) to be used as wake-up patterns.
John, David, can you provide some feedback if the approach is acceptable? I will address Andrew's comment about the user friendliness and allow providing a comma separate list of filter identifiers.
One usability issue with this approach is that one cannot specify wake-on-LAN using WAKE_MAGICSECURE *and* WAKE_FILTER at the same time, since it uses the same location in the ioctl() structure that is being passed. Do you see this as a problem?
Once again we are stuck in this weird situation, a sort of limbo.
On the one hand, I don't want to block your work on the ethtool netlink stuff being done.

Quoted 7 lines

However it is clear that by using netlink attributes, it would be so much cleaner.
I honestly don't know what to say at this time. I wish I had a clear piece of advice and a way for everyone to move forward, and usually I do, but this time I really don't :-/

That's fine, let me submit the first few patches that are per-requisite but don't actually introduce the WAKE_FILTER support. Once Michal's ethtool/netlink work gets merged I can quickly extend that in a way that supports wake-on-LAN using configured filters.

Does the current approach of specifying a bitmask of filters looks reasonable to you though?

Signature · 1 line

Florian

David Miller

maintainer

2018-08-03 21:05 UTC #9 lore raw

Re: [PATCH ethtool] ethtool: Add support for WAKE_FILTER

From: Florian Fainelli <[email protected]>
Date: Fri, 3 Aug 2018 10:57:13 -0700

Quoted 2 lines

Does the current approach of specifying a bitmask of filters looks reasonable to you though?

So, in order to answer that, I need some clarification.

The mask, as I see it, is a bit map of 48 possible positions (SOPASS_MAX * bits_per_byte). How do these bits map to individual rxnfc entries?

Are they locations? If so, how are special locations handled?

What about "special" locations, where the driver and/or hardware are supposed to decide the location based upon the "special" type used?

If you considered the following, and you explained why it won't work, I apologize. But I'm wondering why you just don't find some way to specify this as a boolean of the flow spec in the rxnfc request or similar?

That, at least semantically, seems to avoids several issues. And it is unambiguous what flow rule the wake filter boolean applies to.

Right?

Florian Fainelli

author

2018-08-03 21:56 UTC #10 lore raw

Re: [PATCH ethtool] ethtool: Add support for WAKE_FILTER

Quoted 11 lines · David Miller

From: Florian Fainelli <[email protected]>
Date: Fri, 3 Aug 2018 10:57:13 -0700
Quoted 2 lines
Does the current approach of specifying a bitmask of filters looks reasonable to you though?
So, in order to answer that, I need some clarification.
The mask, as I see it, is a bit map of 48 possible positions (SOPASS_MAX * bits_per_byte). How do these bits map to individual rxnfc entries?

Correct about the size, it is 48-bits, each bit indeed does map to a filter location. So if you programmed a filter a location 1, you would pass 0x2 as the wake-on filter bitmask, etc.

Quoted 6 lines

Are they locations? If so, how are special locations handled?
What about "special" locations, where the driver and/or hardware are supposed to decide the location based upon the "special" type used?

I would not think they require special handling because the process is kind of two step right now:

- first you program the desired filter (special location or not) and you obtain an unique ID back
- second you program the desired filter mask with that ID as a bit position that must be set

So the special location handling was kind of done by the kernel/driver on the first filter insertion and you just pass that unique filter ID around.

The reason why it was done as a two step process was largely because the DSA switch driver, which is the one supporting the filter programming is a discrete driver from the SYSTEM PORT driver which supports the wake-on-filter thing. The two do communicate with one another through the means of the DSA layer though.

Now that I think about it some more, see below, I prefer you approach since it eliminates the "passing that ID around" step.

Quoted 10 lines

If you considered the following, and you explained why it won't work, I apologize. But I'm wondering why you just don't find some way to specify this as a boolean of the flow spec in the rxnfc request or similar?
That, at least semantically, seems to avoids several issues. And it is unambiguous what flow rule the wake filter boolean applies to.
Right?

Yes, it would actually remove the need for having to specify a storage location between user-space and kernel space and we would also be able to valid ahead of time that we are not overflowing the wake-on-LAN filter capacity. For instance, in the current HW, you can program 128 filters through the switch, but only 8 of those could be wake-up capable at the CPU/management (SYSTEM PORT) level.

Let me cook something that does just that and re-post.

Thanks for your feedback!

Signature · 1 line

Florian

David Miller

maintainer

2018-08-03 22:16 UTC #11 lore raw

Re: [PATCH ethtool] ethtool: Add support for WAKE_FILTER

From: Florian Fainelli <[email protected]>
Date: Fri, 3 Aug 2018 12:58:12 -0700

Quoted 3 lines

For instance, in the current HW, you can program 128 filters through the switch, but only 8 of those could be wake-up capable at the CPU/management (SYSTEM PORT) level.

Yes, I noticed this in the driver patches.

Quoted 3 lines

Let me cook something that does just that and re-post.
Thanks for your feedback!

No problem.

Florian Fainelli

author

2018-07-17 16:10 UTC #12 lore raw

PATCH 1/7 net: dsa: bcm_sf2: Allow targeting CPU ports for CFP rules

In reply to Florian Fainelli

ds->enabled_port_mask only contains a bitmask of user-facing enabled ports, we also need to allow programming CFP rules that target CPU ports (e.g: ports 5 and 8).

Signed-off-by Florian Fainelli <[email protected]>

---
 drivers/net/dsa/bcm_sf2_cfp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/net/dsa/bcm_sf2_cfp.c +2 -1

index b89acaee12d4..1a2b4e16aa13 100644
@@ -755,7 +755,8 @@ static int bcm_sf2_cfp_rule_set(struct dsa_switch *ds, int port,
 	port_num = fs->ring_cookie / SF2_NUM_EGRESS_QUEUES;
 
 	if (fs->ring_cookie == RX_CLS_FLOW_DISC ||
-	    !dsa_is_user_port(ds, port_num) ||
+	    !(dsa_is_user_port(ds, port_num) ||
+	    dsa_is_cpu_port(ds, port_num)) ||
 	    port_num >= priv->hw_params.num_ports)
 		return -EINVAL;
 	/*

Signature · 1 line

2.14.1

David Miller

maintainer

2018-07-18 01:31 UTC #13 lore raw

Re: [PATCH net-next 1/7] net: dsa: bcm_sf2: Allow targeting CPU ports for CFP rules

From: Florian Fainelli <[email protected]>
Date: Tue, 17 Jul 2018 08:36:39 -0700

drivers/net/dsa/bcm_sf2_cfp.c static int bcm_sf2_cfp_rule_set(struct dsa_switch *ds, int port, · line 759

Quoted 7 lines

drivers/net/dsa/bcm_sf2_cfp.c +2 -1

@@ -755,7 +755,8 @@ static int bcm_sf2_cfp_rule_set(struct dsa_switch *ds, int port,
 	port_num = fs->ring_cookie / SF2_NUM_EGRESS_QUEUES;
 
 	if (fs->ring_cookie == RX_CLS_FLOW_DISC ||
-	    !dsa_is_user_port(ds, port_num) ||
+	    !(dsa_is_user_port(ds, port_num) ||
+	    dsa_is_cpu_port(ds, port_num)) ||

I think the second new line needs to be indented by two more spaces, but I could be wrong :-)

Florian Fainelli

author

2018-07-17 16:10 UTC #14 lore raw

PATCH 2/7 net: dsa: bcm_sf2: Disable learning while in WoL

In reply to Florian Fainelli

When we are in Wake-on-LAN, we operate with the host sofware not running a network stack, so we want to the switch to flood packets in order to cause a system wake-up when matching specific filters (unicast or multicast). This was not necessary before since we supported Magic Packet which are targeting a broadcast MAC address which the switch already floods.

Signed-off-by Florian Fainelli <[email protected]>

---
 drivers/net/dsa/bcm_sf2.c      | 12 +++++++++++-
 drivers/net/dsa/bcm_sf2_regs.h |  2 ++
 2 files changed, 13 insertions(+), 1 deletion(-)

drivers/net/dsa/bcm_sf2.c +11 -1

index ac96ff40d37e..e0066adcd2f3 100644
@@ -166,6 +166,11 @@ static int bcm_sf2_port_setup(struct dsa_switch *ds, int port,
 	reg &= ~P_TXQ_PSM_VDD(port);
 	core_writel(priv, reg, CORE_MEM_PSM_VDD_CTRL);
 
+	/* Enable learning */
+	reg = core_readl(priv, CORE_DIS_LEARN);
+	reg &= ~BIT(port);
+	core_writel(priv, reg, CORE_DIS_LEARN);
+
 	/* Enable Broadcom tags for that port if requested */
 	if (priv->brcm_tag_mask & BIT(port))
 		b53_brcm_hdr_setup(ds, port);

@@ -222,8 +227,13 @@ static void bcm_sf2_port_disable(struct dsa_switch *ds, int port,
 	struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
 	u32 reg;
 
-	if (priv->wol_ports_mask & (1 << port))
+	/* Disable learning while in WoL mode */
+	if (priv->wol_ports_mask & (1 << port)) {
+		reg = core_readl(priv, CORE_DIS_LEARN);
+		reg |= BIT(port);
+		core_writel(priv, reg, CORE_DIS_LEARN);
 		return;
+	}
 
 	if (port == priv->moca_port)
 		bcm_sf2_port_intr_disable(priv, port);

drivers/net/dsa/bcm_sf2_regs.h +2 -0

index 3ccd5a865dcb..0a1e530d52b7 100644
@@ -168,6 +168,8 @@ enum bcm_sf2_reg_offs {
 #define CORE_SWITCH_CTRL		0x00088
 #define  MII_DUMB_FWDG_EN		(1 << 6)
 
+#define CORE_DIS_LEARN			0x000f0
+
 #define CORE_SFT_LRN_CTRL		0x000f8
 #define  SW_LEARN_CNTL(x)		(1 << (x))

Signature · 1 line

2.14.1

Andrew Lunn

maintainer

2018-07-17 16:28 UTC #15 lore raw

Re: [PATCH net-next 2/7] net: dsa: bcm_sf2: Disable learning while in WoL

Quoted 2 lines · Florian Fainelli

When we are in Wake-on-LAN, we operate with the host sofware not running a network stack, so we want to the switch to flood packets

Hi Florian

Just to be sure...

By flood, you mean from a user port to the CPU. You don't mean flooding between user ports.

Andrew

Florian Fainelli

author

2018-07-17 16:39 UTC #16 lore raw

Re: [PATCH net-next 2/7] net: dsa: bcm_sf2: Disable learning while in WoL

Quoted 10 lines · Andrew Lunn

Quoted 2 lines · Florian Fainelli
When we are in Wake-on-LAN, we operate with the host sofware not running a network stack, so we want to the switch to flood packets
Hi Florian
Just to be sure...
By flood, you mean from a user port to the CPU. You don't mean flooding between user ports.

Yes, flooding from an user-port to the CPU port, that is, behaving like a fully unmanaged switch during WoL.

Signature · 1 line

Florian

Florian Fainelli

author

2018-07-17 16:10 UTC #17 lore raw

PATCH 3/7 net: systemport: Do not re-configure upon WoL interrupt

In reply to Florian Fainelli

We already properly resume from Wake-on-LAN whether such a condition occured or not, no need to process the WoL interrupt for functional changes since that could race with other settings.

Signed-off-by Florian Fainelli <[email protected]>

---
 drivers/net/ethernet/broadcom/bcmsysport.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

drivers/net/ethernet/broadcom/bcmsysport.c +1 -3

index eb890c4b3b2d..f152826f3d06 100644
@@ -1102,10 +1102,8 @@ static irqreturn_t bcm_sysport_rx_isr(int irq, void *dev_id)
 	if (priv->irq0_stat & INTRL2_0_TX_RING_FULL)
 		bcm_sysport_tx_reclaim_all(priv);
 
-	if (priv->irq0_stat & INTRL2_0_MPD) {
+	if (priv->irq0_stat & INTRL2_0_MPD)
 		netdev_info(priv->netdev, "Wake-on-LAN interrupt!\n");
-		bcm_sysport_resume_from_wol(priv);
-	}
 
 	if (!priv->is_lite)
 		goto out;

Signature · 1 line

2.14.1

Florian Fainelli

author

2018-07-17 16:10 UTC #18 lore raw

PATCH 4/7 net: systemport: Create helper to set MPD

In reply to Florian Fainelli

Create a helper function to turn on/off MPD, this will be used to avoid duplicating code as we are going to add additional types of wake-up types.

Signed-off-by Florian Fainelli <[email protected]>

---
 drivers/net/ethernet/broadcom/bcmsysport.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

drivers/net/ethernet/broadcom/bcmsysport.c +13 -7

index f152826f3d06..511caec7030a 100644
@@ -1041,17 +1041,25 @@ static int bcm_sysport_poll(struct napi_struct *napi, int budget)
 	return work_done;
 }
 
-static void bcm_sysport_resume_from_wol(struct bcm_sysport_priv *priv)
+static void mpd_enable_set(struct bcm_sysport_priv *priv, bool enable)
 {
 	u32 reg;
 
+	reg = umac_readl(priv, UMAC_MPD_CTRL);
+	if (enable)
+		reg |= MPD_EN;
+	else
+		reg &= ~MPD_EN;
+	umac_writel(priv, reg, UMAC_MPD_CTRL);
+}
+
+static void bcm_sysport_resume_from_wol(struct bcm_sysport_priv *priv)
+{
 	/* Stop monitoring MPD interrupt */
 	intrl2_0_mask_set(priv, INTRL2_0_MPD);
 
 	/* Clear the MagicPacket detection logic */
-	reg = umac_readl(priv, UMAC_MPD_CTRL);
-	reg &= ~MPD_EN;
-	umac_writel(priv, reg, UMAC_MPD_CTRL);
+	mpd_enable_set(priv, false);
 
 	netif_dbg(priv, wol, priv->netdev, "resumed from WOL\n");
 }

@@ -2447,9 +2455,7 @@ static int bcm_sysport_suspend_to_wol(struct bcm_sysport_priv *priv)
 
 	/* Do not leave the UniMAC RBUF matching only MPD packets */
 	if (!timeout) {
-		reg = umac_readl(priv, UMAC_MPD_CTRL);
-		reg &= ~MPD_EN;
-		umac_writel(priv, reg, UMAC_MPD_CTRL);
+		mpd_enable_set(priv, false);
 		netif_err(priv, wol, ndev, "failed to enter WOL mode\n");
 		return -ETIMEDOUT;
 	}

Signature · 1 line

2.14.1

Florian Fainelli

author

2018-07-17 16:10 UTC #19 lore raw

PATCH 5/7 ethtool: Add WAKE_FILTER bitmask

In reply to Florian Fainelli

Add the ability to specify that a filter, programmed through ethtool::rxnfc will be used as a wake-up source. sopass which is a 48-bit wide storage is used to indicate which filters (as bits) can be used for wake-up.

Signed-off-by Florian Fainelli <[email protected]>

---
 include/uapi/linux/ethtool.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

include/uapi/linux/ethtool.h +2 -1

index 4ca65b56084f..59e35f0ca9eb 100644
@@ -201,7 +201,7 @@ struct ethtool_drvinfo {
  * @supported: Bitmask of %WAKE_* flags for supported Wake-On-Lan modes.
  *	Read-only.
  * @wolopts: Bitmask of %WAKE_* flags for enabled Wake-On-Lan modes.
- * @sopass: SecureOn(tm) password; meaningful only if %WAKE_MAGICSECURE
+ * @sopass: SecureOn(tm) password; meaningful only if %WAKE_MAGICSECURE or %WAKE_FILTER
  *	is set in @wolopts.
  */
 struct ethtool_wolinfo {

@@ -1634,6 +1634,7 @@ static inline int ethtool_validate_duplex(__u8 duplex)
 #define WAKE_ARP		(1 << 4)
 #define WAKE_MAGIC		(1 << 5)
 #define WAKE_MAGICSECURE	(1 << 6) /* only meaningful if WAKE_MAGIC */
+#define WAKE_FILTER		(1 << 7)
 
 /* L2-L4 network traffic flow types */
 #define	TCP_V4_FLOW	0x01	/* hash or spec (tcp_ip4_spec) */

Signature · 1 line

2.14.1

Florian Fainelli

author

2018-07-17 16:10 UTC #20 lore raw

PATCH 6/7 net: systemport: Add support for WAKE_FILTER

In reply to Florian Fainelli

The SYSTEMPORT MAC allows up to 8 filters to be programmed to wake-up from LAN. Verify that we have up to 8 filters and program them to the appropriate RXCHK entries to be matched (along with their masks).

We need to update the entry and exit to Wake-on-LAN mode to keep the RXCHK engine running to match during suspend, but this is otherwise fairly similar to Magic Packet detection.

Signed-off-by Florian Fainelli <[email protected]>

---
 drivers/net/ethernet/broadcom/bcmsysport.c | 111 +++++++++++++++++++++++++----
 drivers/net/ethernet/broadcom/bcmsysport.h |  14 +++-
 2 files changed, 109 insertions(+), 16 deletions(-)

drivers/net/ethernet/broadcom/bcmsysport.c +97 ~~-14~~

index 511caec7030a..8d7ce3df1080 100644
@@ -521,25 +521,31 @@ static void bcm_sysport_get_wol(struct net_device *dev,
 	struct bcm_sysport_priv *priv = netdev_priv(dev);
 	u32 reg;
 
-	wol->supported = WAKE_MAGIC | WAKE_MAGICSECURE;
+	wol->supported = WAKE_MAGIC | WAKE_MAGICSECURE | WAKE_FILTER;
 	wol->wolopts = priv->wolopts;
 
-	if (!(priv->wolopts & WAKE_MAGICSECURE))
-		return;
+	if (priv->wolopts & WAKE_MAGICSECURE) {
+		/* Return the programmed SecureOn password */
+		reg = umac_readl(priv, UMAC_PSW_MS);
+		put_unaligned_be16(reg, &wol->sopass[0]);
+		reg = umac_readl(priv, UMAC_PSW_LS);
+		put_unaligned_be32(reg, &wol->sopass[2]);
+	}
 
-	/* Return the programmed SecureOn password */
-	reg = umac_readl(priv, UMAC_PSW_MS);
-	put_unaligned_be16(reg, &wol->sopass[0]);
-	reg = umac_readl(priv, UMAC_PSW_LS);
-	put_unaligned_be32(reg, &wol->sopass[2]);
+	if (priv->wolopts & WAKE_FILTER)
+		bitmap_copy((unsigned long *)wol->sopass, priv->filters,
+			    WAKE_FILTER_BITS);
 }
 
+
 static int bcm_sysport_set_wol(struct net_device *dev,
 			       struct ethtool_wolinfo *wol)
 {
 	struct bcm_sysport_priv *priv = netdev_priv(dev);
 	struct device *kdev = &priv->pdev->dev;
-	u32 supported = WAKE_MAGIC | WAKE_MAGICSECURE;
+	u32 supported = WAKE_MAGIC | WAKE_MAGICSECURE | WAKE_FILTER;
+	unsigned int index, i = 0;
+	u32 reg;
 
 	if (!device_can_wakeup(kdev))
 		return -ENOTSUPP;

@@ -555,6 +561,32 @@ static int bcm_sysport_set_wol(struct net_device *dev,
 			    UMAC_PSW_LS);
 	}
 
+	/* We support matching up to 8 filters only */
+	if (wol->wolopts & WAKE_FILTER) {
+		bitmap_copy(priv->filters, (unsigned long *)wol->sopass,
+			    WAKE_FILTER_BITS);
+
+		if (bitmap_weight(priv->filters, WAKE_FILTER_BITS) >
+				  RXCHK_BRCM_TAG_MAX) {
+			bitmap_zero(priv->filters, WAKE_FILTER_BITS);
+			return -ENOSPC;
+		}
+
+		if (bitmap_weight(priv->filters, WAKE_FILTER_BITS) == 0)
+			return -EINVAL;
+
+		for_each_set_bit(index, priv->filters, WAKE_FILTER_BITS) {
+			/* Write the index we want to match within the CID field */
+			reg = rxchk_readl(priv, RXCHK_BRCM_TAG(i));
+			reg &= ~(RXCHK_BRCM_TAG_CID_MASK <<
+				 RXCHK_BRCM_TAG_CID_SHIFT);
+			reg |= index << RXCHK_BRCM_TAG_CID_SHIFT;
+			rxchk_writel(priv, reg, RXCHK_BRCM_TAG(i));
+			rxchk_writel(priv, 0xff00ffff, RXCHK_BRCM_TAG_MASK(i));
+			i++;
+		}
+	}
+
 	/* Flag the device and relevant IRQ as wakeup capable */
 	if (wol->wolopts) {
 		device_set_wakeup_enable(kdev, 1);

@@ -1043,7 +1075,7 @@ static int bcm_sysport_poll(struct napi_struct *napi, int budget)
 
 static void mpd_enable_set(struct bcm_sysport_priv *priv, bool enable)
 {
-	u32 reg;
+	u32 reg, bit;
 
 	reg = umac_readl(priv, UMAC_MPD_CTRL);
 	if (enable)

@@ -1051,12 +1083,32 @@ static void mpd_enable_set(struct bcm_sysport_priv *priv, bool enable)
 	else
 		reg &= ~MPD_EN;
 	umac_writel(priv, reg, UMAC_MPD_CTRL);
+
+	if (priv->is_lite)
+		bit = RBUF_ACPI_EN_LITE;
+	else
+		bit = RBUF_ACPI_EN;
+
+	reg = rbuf_readl(priv, RBUF_CONTROL);
+	if (enable)
+		reg |= bit;
+	else
+		reg &= ~bit;
+	rbuf_writel(priv, reg, RBUF_CONTROL);
 }
 
 static void bcm_sysport_resume_from_wol(struct bcm_sysport_priv *priv)
 {
+	u32 reg;
+
 	/* Stop monitoring MPD interrupt */
-	intrl2_0_mask_set(priv, INTRL2_0_MPD);
+	intrl2_0_mask_set(priv, INTRL2_0_MPD | INTRL2_0_BRCM_MATCH_TAG);
+
+	/* Disable RXCHK, active filters and Broadcom tag matching */
+	reg = rxchk_readl(priv, RXCHK_CONTROL);
+	reg &= ~(RXCHK_BRCM_TAG_MATCH_MASK <<
+		 RXCHK_BRCM_TAG_MATCH_SHIFT | RXCHK_EN | RXCHK_BRCM_TAG_EN);
+	rxchk_writel(priv, reg, RXCHK_CONTROL);
 
 	/* Clear the MagicPacket detection logic */
 	mpd_enable_set(priv, false);

@@ -1085,6 +1137,7 @@ static irqreturn_t bcm_sysport_rx_isr(int irq, void *dev_id)
 	struct bcm_sysport_priv *priv = netdev_priv(dev);
 	struct bcm_sysport_tx_ring *txr;
 	unsigned int ring, ring_bit;
+	u32 reg;
 
 	priv->irq0_stat = intrl2_0_readl(priv, INTRL2_CPU_STATUS) &
 			  ~intrl2_0_readl(priv, INTRL2_CPU_MASK_STATUS);

@@ -1111,7 +1164,14 @@ static irqreturn_t bcm_sysport_rx_isr(int irq, void *dev_id)
 		bcm_sysport_tx_reclaim_all(priv);
 
 	if (priv->irq0_stat & INTRL2_0_MPD)
-		netdev_info(priv->netdev, "Wake-on-LAN interrupt!\n");
+		netdev_info(priv->netdev, "Wake-on-LAN (MPD) interrupt!\n");
+
+	if (priv->irq0_stat & INTRL2_0_BRCM_MATCH_TAG) {
+		reg = rxchk_readl(priv, RXCHK_BRCM_TAG_MATCH_STATUS) &
+				  RXCHK_BRCM_TAG_MATCH_MASK;
+		netdev_info(priv->netdev,
+			    "Wake-on-LAN (filters 0x%02x) interrupt!\n", reg);
+	}
 
 	if (!priv->is_lite)
 		goto out;

@@ -2434,16 +2494,39 @@ static int bcm_sysport_suspend_to_wol(struct bcm_sysport_priv *priv)
 {
 	struct net_device *ndev = priv->netdev;
 	unsigned int timeout = 1000;
+	unsigned int index, i = 0;
 	u32 reg;
 
 	/* Password has already been programmed */
 	reg = umac_readl(priv, UMAC_MPD_CTRL);
-	reg |= MPD_EN;
+	if (priv->wolopts & (WAKE_MAGIC | WAKE_MAGICSECURE))
+		reg |= MPD_EN;
 	reg &= ~PSW_EN;
 	if (priv->wolopts & WAKE_MAGICSECURE)
 		reg |= PSW_EN;
 	umac_writel(priv, reg, UMAC_MPD_CTRL);
 
+	if (priv->wolopts & WAKE_FILTER) {
+		/* Turn on ACPI matching to steal packets from RBUF */
+		reg = rbuf_readl(priv, RBUF_CONTROL);
+		if (priv->is_lite)
+			reg |= RBUF_ACPI_EN_LITE;
+		else
+			reg |= RBUF_ACPI_EN;
+		rbuf_writel(priv, reg, RBUF_CONTROL);
+
+		/* Enable RXCHK, active filters and Broadcom tag matching */
+		reg = rxchk_readl(priv, RXCHK_CONTROL);
+		reg &= ~(RXCHK_BRCM_TAG_MATCH_MASK <<
+			 RXCHK_BRCM_TAG_MATCH_SHIFT);
+		for_each_set_bit(index, priv->filters, WAKE_FILTER_BITS) {
+			reg |= BIT(RXCHK_BRCM_TAG_MATCH_SHIFT + i);
+			i++;
+		}
+		reg |= RXCHK_EN | RXCHK_BRCM_TAG_EN;
+		rxchk_writel(priv, reg, RXCHK_CONTROL);
+	}
+
 	/* Make sure RBUF entered WoL mode as result */
 	do {
 		reg = rbuf_readl(priv, RBUF_STATUS);

@@ -2464,7 +2547,7 @@ static int bcm_sysport_suspend_to_wol(struct bcm_sysport_priv *priv)
 	umac_enable_set(priv, CMD_RX_EN, 1);
 
 	/* Enable the interrupt wake-up source */
-	intrl2_0_mask_clear(priv, INTRL2_0_MPD);
+	intrl2_0_mask_clear(priv, INTRL2_0_MPD | INTRL2_0_BRCM_MATCH_TAG);
 
 	netif_dbg(priv, wol, ndev, "entered WOL mode\n");

drivers/net/ethernet/broadcom/bcmsysport.h +12 -2

index d6e5d0cbf3a3..6a64bd8cf787 100644
@@ -11,6 +11,8 @@
 #ifndef __BCM_SYSPORT_H
 #define __BCM_SYSPORT_H
 
+#include <linux/bitmap.h>
+#include <linux/ethtool.h>
 #include <linux/if_vlan.h>
 #include <linux/net_dim.h>

@@ -155,14 +157,18 @@ struct bcm_rsb {
 #define  RXCHK_PARSE_AUTH		(1 << 22)
 
 #define RXCHK_BRCM_TAG0			0x04
-#define RXCHK_BRCM_TAG(i)		((i) * RXCHK_BRCM_TAG0)
+#define RXCHK_BRCM_TAG(i)		((i) * 0x4 + RXCHK_BRCM_TAG0)
 #define RXCHK_BRCM_TAG0_MASK		0x24
-#define RXCHK_BRCM_TAG_MASK(i)		((i) * RXCHK_BRCM_TAG0_MASK)
+#define RXCHK_BRCM_TAG_MASK(i)		((i) * 0x4 + RXCHK_BRCM_TAG0_MASK)
 #define RXCHK_BRCM_TAG_MATCH_STATUS	0x44
 #define RXCHK_ETHERTYPE			0x48
 #define RXCHK_BAD_CSUM_CNTR		0x4C
 #define RXCHK_OTHER_DISC_CNTR		0x50
 
+#define RXCHK_BRCM_TAG_MAX		8
+#define RXCHK_BRCM_TAG_CID_SHIFT	16
+#define RXCHK_BRCM_TAG_CID_MASK		0xff
+
 /* TXCHCK offsets and defines */
 #define SYS_PORT_TXCHK_OFFSET		0x380
 #define TXCHK_PKT_RDY_THRESH		0x00

@@ -185,6 +191,7 @@ struct bcm_rsb {
 #define  RBUF_RSB_SWAP0			(1 << 22)
 #define  RBUF_RSB_SWAP1			(1 << 23)
 #define  RBUF_ACPI_EN			(1 << 23)
+#define  RBUF_ACPI_EN_LITE		(1 << 24)
 
 #define RBUF_PKT_RDY_THRESH		0x04

@@ -524,6 +531,8 @@ struct dma_desc {
 
 #define WORDS_PER_DESC			(sizeof(struct dma_desc) / sizeof(u32))
 
+#define WAKE_FILTER_BITS		(SOPASS_MAX * BITS_PER_BYTE)
+
 /* Rx/Tx common counter group.*/
 struct bcm_sysport_pkt_counters {
 	u32	cnt_64;		/* RO Received/Transmited 64 bytes packet */

@@ -776,6 +785,7 @@ struct bcm_sysport_priv {
 
 	/* Ethtool */
 	u32			msg_enable;
+	DECLARE_BITMAP(filters, SOPASS_MAX * BITS_PER_BYTE);
 
 	struct bcm_sysport_stats64	stats64;

Signature · 1 line

2.14.1

Andrew Lunn

maintainer

2018-07-17 16:48 UTC #21 lore raw

Re: [PATCH net-next 6/7] net: systemport: Add support for WAKE_FILTER

Quoted 38 lines · Florian Fainelli

The SYSTEMPORT MAC allows up to 8 filters to be programmed to wake-up from LAN. Verify that we have up to 8 filters and program them to the appropriate RXCHK entries to be matched (along with their masks).
We need to update the entry and exit to Wake-on-LAN mode to keep the RXCHK engine running to match during suspend, but this is otherwise fairly similar to Magic Packet detection.
Signed-off-by Florian Fainelli <[email protected]>
---
 drivers/net/ethernet/broadcom/bcmsysport.c | 111 +++++++++++++++++++++++++----
 drivers/net/ethernet/broadcom/bcmsysport.h |  14 +++-
 2 files changed, 109 insertions(+), 16 deletions(-)
drivers/net/ethernet/broadcom/bcmsysport.c +8 -5
index 511caec7030a..8d7ce3df1080 100644
@@ -521,25 +521,31 @@ static void bcm_sysport_get_wol(struct net_device *dev,
 	struct bcm_sysport_priv *priv = netdev_priv(dev);
 	u32 reg;
 
-	wol->supported = WAKE_MAGIC | WAKE_MAGICSECURE;
+	wol->supported = WAKE_MAGIC | WAKE_MAGICSECURE | WAKE_FILTER;
 	wol->wolopts = priv->wolopts;
 
-	if (!(priv->wolopts & WAKE_MAGICSECURE))
-		return;
+	if (priv->wolopts & WAKE_MAGICSECURE) {
+		/* Return the programmed SecureOn password */
+		reg = umac_readl(priv, UMAC_PSW_MS);
+		put_unaligned_be16(reg, &wol->sopass[0]);
+		reg = umac_readl(priv, UMAC_PSW_LS);
+		put_unaligned_be32(reg, &wol->sopass[2]);
+	}
 
-	/* Return the programmed SecureOn password */
-	reg = umac_readl(priv, UMAC_PSW_MS);

drivers/net/ethernet/broadcom/bcmsysport.c static void bcm_sysport_get_wol(struct net_device *dev, · line 541

Quoted 10 lines

drivers/net/ethernet/broadcom/bcmsysport.c +4 -3

-	put_unaligned_be16(reg, &wol->sopass[0]);
-	reg = umac_readl(priv, UMAC_PSW_LS);
-	put_unaligned_be32(reg, &wol->sopass[2]);
+	if (priv->wolopts & WAKE_FILTER)
+		bitmap_copy((unsigned long *)wol->sopass, priv->filters,
+			    WAKE_FILTER_BITS);
 }
 
+
 static int bcm_sysport_set_wol(struct net_device *dev,

Two blank lines...

Quoted 12 lines

drivers/net/ethernet/broadcom/bcmsysport.c +3 -1

 			       struct ethtool_wolinfo *wol)
 {
 	struct bcm_sysport_priv *priv = netdev_priv(dev);
 	struct device *kdev = &priv->pdev->dev;
-	u32 supported = WAKE_MAGIC | WAKE_MAGICSECURE;
+	u32 supported = WAKE_MAGIC | WAKE_MAGICSECURE | WAKE_FILTER;
+	unsigned int index, i = 0;
+	u32 reg;
 
 	if (!device_can_wakeup(kdev))
 		return -ENOTSUPP;

drivers/net/ethernet/broadcom/bcmsysport.c static int bcm_sysport_set_wol(struct net_device *dev, · line 567

Quoted 7 lines

drivers/net/ethernet/broadcom/bcmsysport.c +4 -0

@@ -555,6 +561,32 @@ static int bcm_sysport_set_wol(struct net_device *dev,
 			    UMAC_PSW_LS);
 	}
 
+	/* We support matching up to 8 filters only */
+	if (wol->wolopts & WAKE_FILTER) {
+		bitmap_copy(priv->filters, (unsigned long *)wol->sopass,
+			    WAKE_FILTER_BITS);

Shouldn't this be done after to the two checks for errors? Otherwise you have unexpected side effects.

Quoted 11 lines

drivers/net/ethernet/broadcom/bcmsysport.c +11 -0

+
+		if (bitmap_weight(priv->filters, WAKE_FILTER_BITS) >
+				  RXCHK_BRCM_TAG_MAX) {
+			bitmap_zero(priv->filters, WAKE_FILTER_BITS);
+			return -ENOSPC;
+		}
+
+		if (bitmap_weight(priv->filters, WAKE_FILTER_BITS) == 0)
+			return -EINVAL;
+
+		for_each_set_bit(index, priv->filters, WAKE_FILTER_BITS) {

drivers/net/ethernet/broadcom/bcmsysport.c static int bcm_sysport_set_wol(struct net_device *dev, · line 588

Quoted 10 lines

drivers/net/ethernet/broadcom/bcmsysport.c +10 -0

+			/* Write the index we want to match within the CID field */
+			reg = rxchk_readl(priv, RXCHK_BRCM_TAG(i));
+			reg &= ~(RXCHK_BRCM_TAG_CID_MASK <<
+				 RXCHK_BRCM_TAG_CID_SHIFT);
+			reg |= index << RXCHK_BRCM_TAG_CID_SHIFT;
+			rxchk_writel(priv, reg, RXCHK_BRCM_TAG(i));
+			rxchk_writel(priv, 0xff00ffff, RXCHK_BRCM_TAG_MASK(i));
+			i++;
+		}
+	}

How do you disable filters? It looks like you cannot pass all bits set to 0. Also, how do you disable a specific filter? The code above seems to be additive only. There does not appear to be a first write which disables all existing filters before writing the new set of filters.

Andrew

Florian Fainelli

author

2018-07-17 16:59 UTC #22 lore raw

Re: [PATCH net-next 6/7] net: systemport: Add support for WAKE_FILTER

Quoted 69 lines · Andrew Lunn

Quoted 38 lines · Florian Fainelli

The SYSTEMPORT MAC allows up to 8 filters to be programmed to wake-up from LAN. Verify that we have up to 8 filters and program them to the appropriate RXCHK entries to be matched (along with their masks).
We need to update the entry and exit to Wake-on-LAN mode to keep the RXCHK engine running to match during suspend, but this is otherwise fairly similar to Magic Packet detection.
Signed-off-by Florian Fainelli <[email protected]>
---
 drivers/net/ethernet/broadcom/bcmsysport.c | 111 +++++++++++++++++++++++++----
 drivers/net/ethernet/broadcom/bcmsysport.h |  14 +++-
 2 files changed, 109 insertions(+), 16 deletions(-)
drivers/net/ethernet/broadcom/bcmsysport.c +8 -5
index 511caec7030a..8d7ce3df1080 100644
@@ -521,25 +521,31 @@ static void bcm_sysport_get_wol(struct net_device *dev,
 	struct bcm_sysport_priv *priv = netdev_priv(dev);
 	u32 reg;
 
-	wol->supported = WAKE_MAGIC | WAKE_MAGICSECURE;
+	wol->supported = WAKE_MAGIC | WAKE_MAGICSECURE | WAKE_FILTER;
 	wol->wolopts = priv->wolopts;
 
-	if (!(priv->wolopts & WAKE_MAGICSECURE))
-		return;
+	if (priv->wolopts & WAKE_MAGICSECURE) {
+		/* Return the programmed SecureOn password */
+		reg = umac_readl(priv, UMAC_PSW_MS);
+		put_unaligned_be16(reg, &wol->sopass[0]);
+		reg = umac_readl(priv, UMAC_PSW_LS);
+		put_unaligned_be32(reg, &wol->sopass[2]);
+	}
 
-	/* Return the programmed SecureOn password */
-	reg = umac_readl(priv, UMAC_PSW_MS);

drivers/net/ethernet/broadcom/bcmsysport.c static void bcm_sysport_get_wol(struct net_device *dev, · line 541

Quoted 10 lines

drivers/net/ethernet/broadcom/bcmsysport.c +4 -3

-	put_unaligned_be16(reg, &wol->sopass[0]);
-	reg = umac_readl(priv, UMAC_PSW_LS);
-	put_unaligned_be32(reg, &wol->sopass[2]);
+	if (priv->wolopts & WAKE_FILTER)
+		bitmap_copy((unsigned long *)wol->sopass, priv->filters,
+			    WAKE_FILTER_BITS);
 }
 
+
 static int bcm_sysport_set_wol(struct net_device *dev,

Two blank lines...

Quoted 15 lines

drivers/net/ethernet/broadcom/bcmsysport.c +3 -1

 			       struct ethtool_wolinfo *wol)
 {
 	struct bcm_sysport_priv *priv = netdev_priv(dev);
 	struct device *kdev = &priv->pdev->dev;
-	u32 supported = WAKE_MAGIC | WAKE_MAGICSECURE;
+	u32 supported = WAKE_MAGIC | WAKE_MAGICSECURE | WAKE_FILTER;
+	unsigned int index, i = 0;
+	u32 reg;
 
 	if (!device_can_wakeup(kdev))
 		return -ENOTSUPP;

@@ -555,6 +561,32 @@ static int bcm_sysport_set_wol(struct net_device *dev,
 			    UMAC_PSW_LS);
 	}

Quoted 7 lines

drivers/net/ethernet/broadcom/bcmsysport.c static int bcm_sysport_set_wol(struct net_device *dev, · line 567
Quoted 4 lines
drivers/net/ethernet/broadcom/bcmsysport.c +4 -0
+	/* We support matching up to 8 filters only */
+	if (wol->wolopts & WAKE_FILTER) {
+		bitmap_copy(priv->filters, (unsigned long *)wol->sopass,
+			    WAKE_FILTER_BITS);
Shouldn't this be done after to the two checks for errors? Otherwise you have unexpected side effects.

How would you use the bitmap_* routines if you don't copy the bitmap first? Besides, if the bitmap is too wide (next check), we zero it out, so nothing will get programmed if we attempt a Wake-on-LAN suspend (and priv->wolopts is not copied anyway) and the second check would reject a zero bitmap as well.

Quoted 19 lines

drivers/net/ethernet/broadcom/bcmsysport.c +18 -0

+
+		if (bitmap_weight(priv->filters, WAKE_FILTER_BITS) >
+				  RXCHK_BRCM_TAG_MAX) {
+			bitmap_zero(priv->filters, WAKE_FILTER_BITS);
+			return -ENOSPC;
+		}
+
+		if (bitmap_weight(priv->filters, WAKE_FILTER_BITS) == 0)
+			return -EINVAL;
+
+		for_each_set_bit(index, priv->filters, WAKE_FILTER_BITS) {
+			/* Write the index we want to match within the CID field */
+			reg = rxchk_readl(priv, RXCHK_BRCM_TAG(i));
+			reg &= ~(RXCHK_BRCM_TAG_CID_MASK <<
+				 RXCHK_BRCM_TAG_CID_SHIFT);
+			reg |= index << RXCHK_BRCM_TAG_CID_SHIFT;
+			rxchk_writel(priv, reg, RXCHK_BRCM_TAG(i));
+			rxchk_writel(priv, 0xff00ffff, RXCHK_BRCM_TAG_MASK(i));

Quoted 8 lines

drivers/net/ethernet/broadcom/bcmsysport.c static int bcm_sysport_set_wol(struct net_device *dev, · line 588
Quoted 3 lines
drivers/net/ethernet/broadcom/bcmsysport.c +3 -0
+			i++;
+		}
+	}
How do you disable filters? It looks like you cannot pass all bits set to 0. Also, how do you disable a specific filter? The code above seems to be additive only. There does not appear to be a first write which disables all existing filters before writing the new set of filters.

Either you disable WoL entirely (ethtool -s gphy wol d) and then we don't put the hardware in a state that allows it to wake-up the system, or you re-program a different set of filters by re-sending a new bitmask of desired filters. This is not different from how you program/unprogram MagicPacket with SecureOn password.

Signature · 1 line

Florian

Andrew Lunn

maintainer

2018-07-17 17:22 UTC #23 lore raw

Re: [PATCH net-next 6/7] net: systemport: Add support for WAKE_FILTER

Quoted 23 lines

Quoted 15 lines
Quoted 15 lines
drivers/net/ethernet/broadcom/bcmsysport.c +3 -1
 			       struct ethtool_wolinfo *wol)
 {
 	struct bcm_sysport_priv *priv = netdev_priv(dev);
 	struct device *kdev = &priv->pdev->dev;
-	u32 supported = WAKE_MAGIC | WAKE_MAGICSECURE;
+	u32 supported = WAKE_MAGIC | WAKE_MAGICSECURE | WAKE_FILTER;
+	unsigned int index, i = 0;
+	u32 reg;
 
 	if (!device_can_wakeup(kdev))
 		return -ENOTSUPP;
@@ -555,6 +561,32 @@ static int bcm_sysport_set_wol(struct net_device *dev,
 			    UMAC_PSW_LS);
 	}
 
Quoted 7 lines
drivers/net/ethernet/broadcom/bcmsysport.c static int bcm_sysport_set_wol(struct net_device *dev, · line 567
Quoted 4 lines
drivers/net/ethernet/broadcom/bcmsysport.c +4 -0
+	/* We support matching up to 8 filters only */
+	if (wol->wolopts & WAKE_FILTER) {
+		bitmap_copy(priv->filters, (unsigned long *)wol->sopass,
+			    WAKE_FILTER_BITS);
Shouldn't this be done after to the two checks for errors? Otherwise you have unexpected side effects.

Quoted 5 lines

How would you use the bitmap_* routines if you don't copy the bitmap first? Besides, if the bitmap is too wide (next check), we zero it out, so nothing will get programmed if we attempt a Wake-on-LAN suspend (and priv->wolopts is not copied anyway) and the second check would reject a zero bitmap as well.

Zero'ing it is a side effect. get_wol() will now return that no filtered are programmed. However, it appears the hardware is still programmed with the old filters. Maybe there is a

rxchk_writel(priv, 0, RXCHK_BRCM_TAG(i)

hiding in this code somewhere, clearing out the old bits, but i don't see it.

Quoted 29 lines

Quoted 20 lines
Quoted 20 lines
drivers/net/ethernet/broadcom/bcmsysport.c +18 -0
+
+		if (bitmap_weight(priv->filters, WAKE_FILTER_BITS) >
+				  RXCHK_BRCM_TAG_MAX) {
+			bitmap_zero(priv->filters, WAKE_FILTER_BITS);
+			return -ENOSPC;
+		}
+
+		if (bitmap_weight(priv->filters, WAKE_FILTER_BITS) == 0)
+			return -EINVAL;
+
+		for_each_set_bit(index, priv->filters, WAKE_FILTER_BITS) {
+			/* Write the index we want to match within the CID field */
+			reg = rxchk_readl(priv, RXCHK_BRCM_TAG(i));
+			reg &= ~(RXCHK_BRCM_TAG_CID_MASK <<
+				 RXCHK_BRCM_TAG_CID_SHIFT);
+			reg |= index << RXCHK_BRCM_TAG_CID_SHIFT;
+			rxchk_writel(priv, reg, RXCHK_BRCM_TAG(i));
+			rxchk_writel(priv, 0xff00ffff, RXCHK_BRCM_TAG_MASK(i));
Quoted 8 lines
drivers/net/ethernet/broadcom/bcmsysport.c static int bcm_sysport_set_wol(struct net_device *dev, · line 588
Quoted 3 lines
drivers/net/ethernet/broadcom/bcmsysport.c +3 -0
+			i++;
+		}
+	}
How do you disable filters? It looks like you cannot pass all bits set to 0. Also, how do you disable a specific filter? The code above seems to be additive only. There does not appear to be a first write which disables all existing filters before writing the new set of filters.

Quoted 4 lines

Either you disable WoL entirely (ethtool -s gphy wol d) and then we don't put the hardware in a state that allows it to wake-up the system, or you re-program a different set of filters by re-sending a new bitmask of desired filters.

This appears to be read-modify-write:

Quoted 5 lines

Quoted 5 lines
drivers/net/ethernet/broadcom/bcmsysport.c static int bcm_sysport_set_wol(struct net_device *dev, · line 584
Quoted 5 lines
drivers/net/ethernet/broadcom/bcmsysport.c +5 -0
+			reg = rxchk_readl(priv, RXCHK_BRCM_TAG(i));
+			reg &= ~(RXCHK_BRCM_TAG_CID_MASK <<
+				 RXCHK_BRCM_TAG_CID_SHIFT);
+			reg |= index << RXCHK_BRCM_TAG_CID_SHIFT;
+			rxchk_writel(priv, reg, RXCHK_BRCM_TAG(i));

It looks like you can add more bits, but i don't see any way to clear bits. As i said above, there might be an initial write of 0, but i cannot see it. The obvious place for it would be just before the for_each_set_bit(), or at the beginning of the function.

Andrew

Florian Fainelli

author

2018-07-17 17:30 UTC #24 lore raw

Re: [PATCH net-next 6/7] net: systemport: Add support for WAKE_FILTER

Quoted 29 lines · Andrew Lunn

Quoted 23 lines
Quoted 15 lines
Quoted 15 lines
drivers/net/ethernet/broadcom/bcmsysport.c +3 -1
 			       struct ethtool_wolinfo *wol)
 {
 	struct bcm_sysport_priv *priv = netdev_priv(dev);
 	struct device *kdev = &priv->pdev->dev;
-	u32 supported = WAKE_MAGIC | WAKE_MAGICSECURE;
+	u32 supported = WAKE_MAGIC | WAKE_MAGICSECURE | WAKE_FILTER;
+	unsigned int index, i = 0;
+	u32 reg;
 
 	if (!device_can_wakeup(kdev))
 		return -ENOTSUPP;
@@ -555,6 +561,32 @@ static int bcm_sysport_set_wol(struct net_device *dev,
 			    UMAC_PSW_LS);
 	}
 
Quoted 7 lines
drivers/net/ethernet/broadcom/bcmsysport.c static int bcm_sysport_set_wol(struct net_device *dev, · line 567
Quoted 4 lines
drivers/net/ethernet/broadcom/bcmsysport.c +4 -0
+	/* We support matching up to 8 filters only */
+	if (wol->wolopts & WAKE_FILTER) {
+		bitmap_copy(priv->filters, (unsigned long *)wol->sopass,
+			    WAKE_FILTER_BITS);
Shouldn't this be done after to the two checks for errors? Otherwise you have unexpected side effects.
Quoted 5 lines
How would you use the bitmap_* routines if you don't copy the bitmap first? Besides, if the bitmap is too wide (next check), we zero it out, so nothing will get programmed if we attempt a Wake-on-LAN suspend (and priv->wolopts is not copied anyway) and the second check would reject a zero bitmap as well.

Quoted 8 lines

Zero'ing it is a side effect. get_wol() will now return that no filtered are programmed. However, it appears the hardware is still programmed with the old filters. Maybe there is a
rxchk_writel(priv, 0, RXCHK_BRCM_TAG(i)
hiding in this code somewhere, clearing out the old bits, but i don't see it.

It is not necessary to clear those registers for a number of reasons:

- they are only active if the corresponding bit to enable those is also programmed in RXHCK_CONTROL, which is only done during bcm_sysport_suspend_to_wol(), though I suppose for safety one could be moving the RXCHK_BRCM_TAG_MATCH_MASK clearing outside of the WAKE_FILTER check though again, not necessary because:
- HW starts with those bits cleared
- if you entered WoL once with any of those filters, we would be
clearing those bits again during WoL resume

- if WoL is disabled, we don't even enable network ports to forward traffic and remotely allow a packet to enter the switch (see drivers/net/dsa/bcm_sf2.c)

Quoted 37 lines

Quoted 30 lines
Quoted 21 lines
Quoted 21 lines
drivers/net/ethernet/broadcom/bcmsysport.c +18 -0
+
+		if (bitmap_weight(priv->filters, WAKE_FILTER_BITS) >
+				  RXCHK_BRCM_TAG_MAX) {
+			bitmap_zero(priv->filters, WAKE_FILTER_BITS);
+			return -ENOSPC;
+		}
+
+		if (bitmap_weight(priv->filters, WAKE_FILTER_BITS) == 0)
+			return -EINVAL;
+
+		for_each_set_bit(index, priv->filters, WAKE_FILTER_BITS) {
+			/* Write the index we want to match within the CID field */
+			reg = rxchk_readl(priv, RXCHK_BRCM_TAG(i));
+			reg &= ~(RXCHK_BRCM_TAG_CID_MASK <<
+				 RXCHK_BRCM_TAG_CID_SHIFT);
+			reg |= index << RXCHK_BRCM_TAG_CID_SHIFT;
+			rxchk_writel(priv, reg, RXCHK_BRCM_TAG(i));
+			rxchk_writel(priv, 0xff00ffff, RXCHK_BRCM_TAG_MASK(i));
Quoted 8 lines
drivers/net/ethernet/broadcom/bcmsysport.c static int bcm_sysport_set_wol(struct net_device *dev, · line 588
Quoted 3 lines
drivers/net/ethernet/broadcom/bcmsysport.c +3 -0
+			i++;
+		}
+	}
How do you disable filters? It looks like you cannot pass all bits set to 0. Also, how do you disable a specific filter? The code above seems to be additive only. There does not appear to be a first write which disables all existing filters before writing the new set of filters.
Quoted 4 lines
Either you disable WoL entirely (ethtool -s gphy wol d) and then we don't put the hardware in a state that allows it to wake-up the system, or you re-program a different set of filters by re-sending a new bitmask of desired filters.
This appears to be read-modify-write:

Quoted 10 lines

Quoted 5 lines
Quoted 5 lines
drivers/net/ethernet/broadcom/bcmsysport.c static int bcm_sysport_set_wol(struct net_device *dev, · line 584
Quoted 5 lines
drivers/net/ethernet/broadcom/bcmsysport.c +5 -0
+			reg = rxchk_readl(priv, RXCHK_BRCM_TAG(i));
+			reg &= ~(RXCHK_BRCM_TAG_CID_MASK <<
+				 RXCHK_BRCM_TAG_CID_SHIFT);
+			reg |= index << RXCHK_BRCM_TAG_CID_SHIFT;
+			rxchk_writel(priv, reg, RXCHK_BRCM_TAG(i));
It looks like you can add more bits, but i don't see any way to clear bits. As i said above, there might be an initial write of 0, but i cannot see it. The obvious place for it would be just before the for_each_set_bit(), or at the beginning of the function.

We are only programming the HW to be matching bits [23:16] and with a corresponding mask of 0xff00_ffff so even if these bits contained garbage, they would not be matched by the HW.

Signature · 1 line

Florian

Andrew Lunn

maintainer

2018-07-17 17:40 UTC #25 lore raw

Re: [PATCH net-next 6/7] net: systemport: Add support for WAKE_FILTER

Quoted 13 lines

drivers/net/ethernet/broadcom/bcmsysport.c +13 -0

+		if (bitmap_weight(priv->filters, WAKE_FILTER_BITS) >
+				  RXCHK_BRCM_TAG_MAX) {
+			bitmap_zero(priv->filters, WAKE_FILTER_BITS);
+			return -ENOSPC;
+		}
+
+		if (bitmap_weight(priv->filters, WAKE_FILTER_BITS) == 0)
+			return -EINVAL;
+
+		for_each_set_bit(index, priv->filters, WAKE_FILTER_BITS) {
+			/* Write the index we want to match within the CID field */
+			reg = rxchk_readl(priv, RXCHK_BRCM_TAG(i));
+			reg &= ~(RXCHK_BRCM_TAG_CID_MASK <<

Quoted 8 lines

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
drivers/net/ethernet/broadcom/bcmsysport.c static int bcm_sysport_set_wol(struct net_device *dev, · line 588
Quoted 7 lines
drivers/net/ethernet/broadcom/bcmsysport.c +7 -0
+				 RXCHK_BRCM_TAG_CID_SHIFT);
+			reg |= index << RXCHK_BRCM_TAG_CID_SHIFT;
+			rxchk_writel(priv, reg, RXCHK_BRCM_TAG(i));
+			rxchk_writel(priv, 0xff00ffff, RXCHK_BRCM_TAG_MASK(i));
+			i++;
+		}
+	}

Just to convince me, can you dump the contents of reg. And execute the commands:

ethtool -s gphy wol f filters 0x1
ethtool -s gphy wol f filters 0x2

After these two commands, we expect only one filter bit to be set, bit
1. Filter Bit 0 should of been cleared when the second command was executed.

Andrew

Florian Fainelli

author

2018-07-18 09:52 UTC #26 lore raw

Re: [PATCH net-next 6/7] net: systemport: Add support for WAKE_FILTER

Quoted 25 lines · Andrew Lunn

Quoted 13 lines
Quoted 13 lines
Quoted 13 lines
Quoted 13 lines
Quoted 13 lines
drivers/net/ethernet/broadcom/bcmsysport.c +13 -0
+		if (bitmap_weight(priv->filters, WAKE_FILTER_BITS) >
+				  RXCHK_BRCM_TAG_MAX) {
+			bitmap_zero(priv->filters, WAKE_FILTER_BITS);
+			return -ENOSPC;
+		}
+
+		if (bitmap_weight(priv->filters, WAKE_FILTER_BITS) == 0)
+			return -EINVAL;
+
+		for_each_set_bit(index, priv->filters, WAKE_FILTER_BITS) {
+			/* Write the index we want to match within the CID field */
+			reg = rxchk_readl(priv, RXCHK_BRCM_TAG(i));
+			reg &= ~(RXCHK_BRCM_TAG_CID_MASK <<
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
drivers/net/ethernet/broadcom/bcmsysport.c static int bcm_sysport_set_wol(struct net_device *dev, · line 588
Quoted 7 lines
drivers/net/ethernet/broadcom/bcmsysport.c +7 -0
+				 RXCHK_BRCM_TAG_CID_SHIFT);
+			reg |= index << RXCHK_BRCM_TAG_CID_SHIFT;
+			rxchk_writel(priv, reg, RXCHK_BRCM_TAG(i));
+			rxchk_writel(priv, 0xff00ffff, RXCHK_BRCM_TAG_MASK(i));
+			i++;
+		}
+	}
Just to convince me, can you dump the contents of reg. And execute the commands:

Quoted 6 lines

ethtool -s gphy wol f filters 0x1
ethtool -s gphy wol f filters 0x2
After these two commands, we expect only one filter bit to be set, bit
1. Filter Bit 0 should of been cleared when the second command was executed.

In both of your examples, only one bit is set, what will change is the value being programmed to RXHCK_BRCM_TAG(i), which will be either 0, or 1, but the value programmed to RXCHK_CONTROL as far as which filter is enabled will be the same because we can use filter position 0.

What the code basically does is look at how many bits are set in the filters bitmap, and then it starts populating the filters from filter 0 up to filter 7 with the value of the bit.

Dumps below are:
<physiscal offset>,<value>,<constant name>

ethtool -s gphy wol f filters 0x1:

0x09400300,0x01d91019,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_CONTROL 0x09400304,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_0 0x09400308,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_1 0x0940030c,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_2 0x09400310,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_3 0x09400314,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_4 0x09400318,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_5 0x0940031c,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_6 0x09400320,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_7 0x09400324,0xff00ffff,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_0 0x09400328,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_1 0x0940032c,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_2 0x09400330,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_3 0x09400334,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_4 0x09400338,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_5 0x0940033c,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_6 0x09400340,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_7 0x09400344,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MATCH_STATUS 0x09400348,0x88a88100,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_ETHERTYPE 0x0940034c,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BAD_CHECKSUM_PACKET_DISCARD_COUNTER 0x09400350,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_OTHER_PACKET_DISCARD_COUNTER

ethtool -s gphy wol f filters 0x2:

0x09400300,0x01d91019,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_CONTROL 0x09400304,0x00010000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_0 0x09400308,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_1 0x0940030c,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_2 0x09400310,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_3 0x09400314,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_4 0x09400318,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_5 0x0940031c,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_6 0x09400320,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_7 0x09400324,0xff00ffff,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_0 0x09400328,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_1 0x0940032c,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_2 0x09400330,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_3 0x09400334,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_4 0x09400338,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_5 0x0940033c,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_6 0x09400340,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_7 0x09400344,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MATCH_STATUS 0x09400348,0x88a88100,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_ETHERTYPE 0x0940034c,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BAD_CHECKSUM_PACKET_DISCARD_COUNTER 0x09400350,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_OTHER_PACKET_DISCARD_COUNTER

As you can see, the only thing that changes here is that we re-used BRCM_TAG_0 but programmed a different CID value in there which is the bit position within the filter.

ethtool -s gphy wol f filters 0x3

0x09400300,0x01d91039,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_CONTROL 0x09400304,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_0 0x09400308,0x00010000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_1 0x0940030c,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_2 0x09400310,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_3 0x09400314,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_4 0x09400318,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_5 0x0940031c,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_6 0x09400320,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_7 0x09400324,0xff00ffff,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_0 0x09400328,0xff00ffff,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_1 0x0940032c,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_2 0x09400330,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_3 0x09400334,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_4 0x09400338,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_5 0x0940033c,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_6 0x09400340,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_7 0x09400344,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MATCH_STATUS 0x09400348,0x88a88100,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_ETHERTYPE 0x0940034c,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BAD_CHECKSUM_PACKET_DISCARD_COUNTER 0x09400350,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_OTHER_PACKET_DISCARD_COUNTER

Here we are using two filters, which gets reflected by programming BRCM_TAG_0 and BRCM_TAG_1 and setting the bitmask of enabled filters in RXHCK_CONTROL (0x30)

ethtool -s gphy wol f filters 0x1

0x09400300,0x01d91019,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_CONTROL 0x09400304,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_0 0x09400308,0x00010000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_1 0x0940030c,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_2 0x09400310,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_3 0x09400314,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_4 0x09400318,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_5 0x0940031c,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_6 0x09400320,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_7 0x09400324,0xff00ffff,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_0 0x09400328,0xff00ffff,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_1 0x0940032c,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_2 0x09400330,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_3 0x09400334,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_4 0x09400338,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_5 0x0940033c,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_6 0x09400340,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_7 0x09400344,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MATCH_STATUS 0x09400348,0x88a88100,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_ETHERTYPE 0x0940034c,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BAD_CHECKSUM_PACKET_DISCARD_COUNTER 0x09400350,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_OTHER_PACKET_DISCARD_COUNTER

Here we left the filter that we previously programmed into BRCM_TAG_1, but as you can see from RHCHK_CONTROL set to 0x10, we are not activating it, so it will not be part of the wake-up process.

Register programming is always a bit expensive, which is why the code does not try to clear previously programmed filters because we have this active/deactive bit instead which is more efficient to manage.

Hope this is convincing you :)

Signature · 1 line

Florian

Andrew Lunn

maintainer

2018-07-19 23:10 UTC #27 lore raw

Re: [PATCH net-next 6/7] net: systemport: Add support for WAKE_FILTER

Quoted 8 lines

In both of your examples, only one bit is set, what will change is the value being programmed to RXHCK_BRCM_TAG(i), which will be either 0, or 1, but the value programmed to RXCHK_CONTROL as far as which filter is enabled will be the same because we can use filter position 0.
What the code basically does is look at how many bits are set in the filters bitmap, and then it starts populating the filters from filter 0 up to filter 7 with the value of the bit.

O.K. Now it get it. Sorry for being so slow.

Andrew

Florian Fainelli

author

2018-07-20 10:22 UTC #28 lore raw

Re: [PATCH net-next 6/7] net: systemport: Add support for WAKE_FILTER

Quoted 10 lines · Andrew Lunn

Quoted 8 lines
In both of your examples, only one bit is set, what will change is the value being programmed to RXHCK_BRCM_TAG(i), which will be either 0, or 1, but the value programmed to RXCHK_CONTROL as far as which filter is enabled will be the same because we can use filter position 0.
What the code basically does is look at how many bits are set in the filters bitmap, and then it starts populating the filters from filter 0 up to filter 7 with the value of the bit.
O.K. Now it get it. Sorry for being so slow.

No worries, thanks for reviewing these changes. I have made the requested ethtool changes and will post them when I return from vacation on July 29th.

Thanks Andrew!

Signature · 1 line

Florian

Florian Fainelli

author

2018-07-17 16:10 UTC #29 lore raw

PATCH 7/7 net: dsa: bcm_sf2: Support WAKE_FILTER

In reply to Florian Fainelli

Propagate wol->sopass when WAKE_FILTER is set since that contains the bitmap of filters to be enabled for wake-up.

Signed-off-by Florian Fainelli <[email protected]>

---
 drivers/net/dsa/bcm_sf2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/net/dsa/bcm_sf2.c +1 -1

index e0066adcd2f3..ca562cf6ffd2 100644
@@ -741,7 +741,7 @@ static void bcm_sf2_sw_get_wol(struct dsa_switch *ds, int port,
 	wol->supported = pwol.supported;
 	memset(&wol->sopass, 0, sizeof(wol->sopass));
 
-	if (pwol.wolopts & WAKE_MAGICSECURE)
+	if (pwol.wolopts & (WAKE_MAGICSECURE | WAKE_FILTER))
 		memcpy(&wol->sopass, pwol.sopass, sizeof(wol->sopass));
 
 	if (priv->wol_ports_mask & (1 << port))

Signature · 1 line

2.14.1

Andrew Lunn

maintainer

2018-07-17 16:20 UTC #30 lore raw

In reply to Florian Fainelli

Quoted 12 lines · Florian Fainelli

Hi all,
This patch series adds support for allowing Wake-on-LAN to wake-up the system using configurable filters. This is particular useful in the context of Android where wake on MDNS is a requirement.
We support this by using the bcm_sf2 Compact Field Processor (CFP) which supports matching packets and tagging them with an unique identifier (Classification ID) that is added in each packet being matched through the use of Broadcom tags. The SYSTEMPORT MAC attached to that switch is then used to match that unique identifier and trigger a system wake-up event.

Quoted 7 lines

Last patch is the ethtool modifications to support that feature.
Example:
ethtool --config-nfc gphy flow-type udp4 src-ip 192.168.1.1 dst-ip 192.168.1.32 \
	src-port 1234 dst-port 5678 action 64
Added rule with ID 1

Hi Florian

What is action 64?

Quoted 1 line

ethtool -s gphy wol f filters 0x2

What does this 0x2 represent?

These magic numbers are not so nice.

Andrew

Florian Fainelli

author

2018-07-17 16:40 UTC #31 lore raw

Quoted 17 lines · Andrew Lunn

Quoted 16 lines · Florian Fainelli
Hi all,
This patch series adds support for allowing Wake-on-LAN to wake-up the system using configurable filters. This is particular useful in the context of Android where wake on MDNS is a requirement.
We support this by using the bcm_sf2 Compact Field Processor (CFP) which supports matching packets and tagging them with an unique identifier (Classification ID) that is added in each packet being matched through the use of Broadcom tags. The SYSTEMPORT MAC attached to that switch is then used to match that unique identifier and trigger a system wake-up event.
Last patch is the ethtool modifications to support that feature.
Example:

Quoted 7 lines

Quoted 3 lines
ethtool --config-nfc gphy flow-type udp4 src-ip 192.168.1.1 dst-ip 192.168.1.32 \
	src-port 1234 dst-port 5678 action 64
Added rule with ID 1
Hi Florian
What is action 64?

There are 8 egress queues per port, and we want to target port 8 here. Number of queues is discoverable through sysfs by scanning /sys/class/net/gphy/queues/tx-*. CPU port number is fixed at 8, we don't have a mechanism AFAICT to expose that to users, because, of course, we don't expose the CPU port with DSA.

Quoted 4 lines

Quoted 2 lines
ethtool -s gphy wol f filters 0x2
What does this 0x2 represent?

0x2 = bit 1 is set, which corresponds to the filter ID that was returned from the previous ethtool::rxnfc command invocation. If ethtool --config-nfc returned 3, then we would have used filters 0x8, etc.

Signature · 1 line

Florian

Andrew Lunn

maintainer

2018-07-17 16:55 UTC #32 lore raw

Quoted 7 lines

Quoted 3 lines
Quoted 1 line
ethtool -s gphy wol f filters 0x2
What does this 0x2 represent?
0x2 = bit 1 is set, which corresponds to the filter ID that was returned from the previous ethtool::rxnfc command invocation. If ethtool --config-nfc returned 3, then we would have used filters 0x8, etc.

It would be a simpler for the user if you could pass the filter IDs as a list, and let ethtool do the shift and OR.

Andrew

Florian Fainelli

author

2018-07-17 17:02 UTC #33 lore raw

Quoted 10 lines · Andrew Lunn

Quoted 7 lines
Quoted 3 lines
Quoted 1 line
ethtool -s gphy wol f filters 0x2
What does this 0x2 represent?
0x2 = bit 1 is set, which corresponds to the filter ID that was returned from the previous ethtool::rxnfc command invocation. If ethtool --config-nfc returned 3, then we would have used filters 0x8, etc.
It would be a simpler for the user if you could pass the filter IDs as a list, and let ethtool do the shift and OR.

Sure, I can definitively add that, is a comma separator okay with you for that? E.g:

ethtool -s gphy wol f filters 0,1,2,3,4,5,6,7

Signature · 1 line

Florian

Andrew Lunn

maintainer

2018-07-17 17:25 UTC #34 lore raw

Quoted 11 lines · Florian Fainelli

Quoted 8 lines · Andrew Lunn
Quoted 7 lines
Quoted 3 lines
Quoted 1 line
ethtool -s gphy wol f filters 0x2
What does this 0x2 represent?
0x2 = bit 1 is set, which corresponds to the filter ID that was returned from the previous ethtool::rxnfc command invocation. If ethtool --config-nfc returned 3, then we would have used filters 0x8, etc.

Quoted 7 lines

Quoted 2 lines
It would be a simpler for the user if you could pass the filter IDs as a list, and let ethtool do the shift and OR.
Sure, I can definitively add that, is a comma separator okay with you for that? E.g:
ethtool -s gphy wol f filters 0,1,2,3,4,5,6,7

Hi Florian

That looks good. But maybe check the man page for ethtool and see if there is an established convention.

Andrew