Netdev List
 help / color / mirror / Atom feed
* [PATCH ethtool 3/3] ethtool: Add support for action value -2 (wake-up filter)
From: Florian Fainelli @ 2018-08-08 18:22 UTC (permalink / raw)
  To: netdev; +Cc: linville, andrew, davem, Florian Fainelli
In-Reply-To: <20180808182211.24921-1-f.fainelli@gmail.com>

Add the ability to program special filters using ethtool::rxnfc which
are meant to be used for wake-up purposes (in conjuction with
WAKE_FILTER) using the special action value: -2 (RX_CLS_FLOW_WAKE).

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 rxclass.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/rxclass.c b/rxclass.c
index 42d122d1ed86..79972651e706 100644
--- a/rxclass.c
+++ b/rxclass.c
@@ -251,7 +251,11 @@ static void rxclass_print_nfc_rule(struct ethtool_rx_flow_spec *fsp,
 	if (fsp->flow_type & FLOW_RSS)
 		fprintf(stdout, "\tRSS Context ID: %u\n", rss_context);
 
-	if (fsp->ring_cookie != RX_CLS_FLOW_DISC) {
+	if (fsp->ring_cookie == RX_CLS_FLOW_DISC) {
+		fprintf(stdout, "\tAction: Drop\n");
+	} else if (fsp->ring_cookie == RX_CLS_FLOW_WAKE) {
+		fprintf(stdout, "\tAction: Wake-on-LAN\n");
+	} else {
 		u64 vf = ethtool_get_flow_spec_ring_vf(fsp->ring_cookie);
 		u64 queue = ethtool_get_flow_spec_ring(fsp->ring_cookie);
 
@@ -266,8 +270,6 @@ static void rxclass_print_nfc_rule(struct ethtool_rx_flow_spec *fsp,
 		else
 			fprintf(stdout, "\tAction: Direct to queue %llu\n",
 				queue);
-	} else {
-		fprintf(stdout, "\tAction: Drop\n");
 	}
 
 	fprintf(stdout, "\n");
-- 
2.17.1

^ permalink raw reply related

* [PATCH iproute2-next] Add SKB Priority qdisc support in tc(8)
From: Nishanth Devarajan @ 2018-08-08 18:24 UTC (permalink / raw)
  To: stephen, dsahern; +Cc: netdev, doucette, michel

sch_skbprio is a qdisc that prioritizes packets according to their skb->priority
field. Under congestion, it drops already-enqueued lower priority packets to
make space available for higher priority packets. Skbprio was conceived as a
solution for denial-of-service defenses that need to route packets with
different priorities as a means to overcome DoS attacks.

Signed-off-by: Nishanth Devarajan <ndev2021@gmail.com>
Reviewed-by: Michel Machado <michel@digirati.com.br>
---
 include/uapi/linux/pkt_sched.h |  7 ++++
 man/man8/tc-skbprio.8          | 70 ++++++++++++++++++++++++++++++++++++
 tc/Makefile                    |  1 +
 tc/q_skbprio.c                 | 81 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 159 insertions(+)
 create mode 100644 man/man8/tc-skbprio.8
 create mode 100644 tc/q_skbprio.c

diff --git a/include/uapi/linux/pkt_sched.h b/include/uapi/linux/pkt_sched.h
index 37b5096..81af99e 100644
--- a/include/uapi/linux/pkt_sched.h
+++ b/include/uapi/linux/pkt_sched.h
@@ -124,6 +124,12 @@ struct tc_fifo_qopt {
 	__u32	limit;	/* Queue length: bytes for bfifo, packets for pfifo */
 };
 
+/* SKBPRIO section */
+
+struct tc_skbprio_qopt {
+	__u32	limit;		/* Queue length in packets. */
+};
+
 /* PRIO section */
 
 #define TCQ_PRIO_BANDS	16
@@ -256,6 +262,7 @@ struct tc_red_qopt {
 #define TC_RED_ECN		1
 #define TC_RED_HARDDROP		2
 #define TC_RED_ADAPTATIVE	4
+#define TC_RED_OFFLOADED	8
 };
 
 struct tc_red_xstats {
diff --git a/man/man8/tc-skbprio.8 b/man/man8/tc-skbprio.8
new file mode 100644
index 0000000..15a8524
--- /dev/null
+++ b/man/man8/tc-skbprio.8
@@ -0,0 +1,70 @@
+.TH SKBPRIO 8 "8 August 2018" "iproute2" "Linux"
+.SH NAME
+skbprio \- SKB Priority Queue
+
+.SH SYNOPSIS
+.B tc qdisc ... add skbprio
+.B [ limit
+packets
+.B ]
+
+.SH DESCRIPTION
+SKB Priority Queue is a queueing discipline intended to prioritize
+the most important packets during a denial-of-service (
+.B DoS
+) attack. The priority of a packet is given by
+.B skb->priority
+, where a higher value places the packet closer to the exit of the queue. When
+the queue is full, the lowest priority packet in the queue is dropped to make
+room for the packet to be added if it has higher priority. If the packet to be
+added has lower priority than all packets in the queue, it is dropped.
+
+Without SKB priority queue, queue length limits must be imposed
+on individual sub-queues, and there is no straightforward way to enforce
+a global queue length limit across all priorities. SKBprio queue enforces a
+global queue length limit while not restricting the lengths of individual
+sub-queues.
+
+While SKB Priority Queue is agnostic to how
+.B skb->priority
+is assigned. A typical use case is to copy
+the 6-bit DS field of IPv4 and IPv6 packets using
+.BR tc-skbedit (8)
+. If
+.B skb->priority
+is greater or equal to 64, the priority is assumed to be 63.
+Priorities less than 64 are taken at face value.
+
+SKB Priority Queue enables routers to locally decide which
+packets to drop under a DoS attack.
+Priorities should be assigned to packets such that the higher the priority,
+the more expected behavior a source shows.
+So sources have an incentive to play by the rules.
+
+.SH ALGORITHM
+
+Skbprio maintains 64 lists (priorities go from 0 to 63).
+When a packet is enqueued, it gets inserted at the
+.B tail
+of its priority list. When a packet needs to be sent out to the network, it is
+taken from the head of the highest priority list. When the queue is full,
+the packet at the tail of the lowest priority list is dropped to serve the
+ingress packet - if it is of higher priority, otherwise the ingress packet is
+dropped. This algorithm allocates as much bandwidth as possible to high
+priority packets, while only servicing low priority packets when
+there is enough bandwidth.
+
+.SH PARAMETERS
+.TP
+limit
+Maximum queue size specified in packets. It defaults to 64.
+The range for this parameter is [0, UINT32_MAX].
+
+.SH SEE ALSO
+.BR tc-prio (8),
+.BR tc-skbedit (8)
+
+.SH AUTHORS
+Nishanth Devarajan <devarajn@uci.edu>, Michel Machado <michel@digirati.com.br>
+
+This manpage maintained by Bert Hubert <ahu@ds9a.nl>
diff --git a/tc/Makefile b/tc/Makefile
index dfd0026..7646105 100644
--- a/tc/Makefile
+++ b/tc/Makefile
@@ -71,6 +71,7 @@ TCMODULES += q_clsact.o
 TCMODULES += e_bpf.o
 TCMODULES += f_matchall.o
 TCMODULES += q_cbs.o
+TCMODULES += q_skbprio.o
 
 TCSO :=
 ifeq ($(TC_CONFIG_ATM),y)
diff --git a/tc/q_skbprio.c b/tc/q_skbprio.c
new file mode 100644
index 0000000..a2a5077
--- /dev/null
+++ b/tc/q_skbprio.c
@@ -0,0 +1,81 @@
+/*
+ * q_skbprio.c		SKB PRIORITY QUEUE.
+ *
+ *		This program is free software; you can redistribute it and/or
+ *		modify it under the terms of the GNU General Public License
+ *		as published by the Free Software Foundation; either version
+ *		2 of the License, or (at your option) any later version.
+ *
+ * Authors:	Nishanth Devarajan, <ndev_2021@gmail.com>
+ *
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <syslog.h>
+#include <fcntl.h>
+#include <sys/socket.h>
+#include <netinet/in.h>
+#include <arpa/inet.h>
+#include <string.h>
+
+#include "utils.h"
+#include "tc_util.h"
+
+static void explain(void)
+{
+	fprintf(stderr, "Usage: ... <skbprio> [ limit NUMBER ]\n");
+}
+
+static int skbprio_parse_opt(struct qdisc_util *qu, int argc, char **argv,
+			     struct nlmsghdr *n, const char *dev)
+{
+	int ok = 0;
+	struct tc_skbprio_qopt opt = {};
+
+	while (argc > 0) {
+		if (strcmp(*argv, "limit") == 0) {
+			NEXT_ARG();
+			if (get_size(&opt.limit, *argv)) {
+				fprintf(stderr, "%s: Illegal value for \"limit\": \"%s\"\n", qu->id, *argv);
+				return -1;
+			}
+			ok++;
+		}
+		else if (strcmp(*argv, "help") == 0) {
+			explain();
+			return -1;
+		} else {
+			fprintf(stderr, "%s: unknown parameter \"%s\"\n", qu->id, *argv);
+			explain();
+			return -1;
+		}
+		argc--; argv++;
+	}
+
+	if (ok)
+		addattr_l(n, 1024, TCA_OPTIONS, &opt, sizeof(opt));
+	return 0;
+}
+
+static int skbprio_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
+{
+	struct tc_skbprio_qopt *qopt;
+
+	if (opt == NULL)
+		return 0;
+
+	if (RTA_PAYLOAD(opt)  < sizeof(*qopt))
+		return -1;
+	qopt = RTA_DATA(opt);
+	fprintf(f, "limit %u ", qopt->limit);
+	return 0;
+}
+
+
+struct qdisc_util skbprio_qdisc_util = {
+	.id = "skbprio",
+	.parse_qopt = skbprio_parse_opt,
+	.print_qopt = skbprio_print_opt,
+};
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH ethtool 2/3] ethtool: Add support for WAKE_FILTER (WoL using filters)
From: Florian Fainelli @ 2018-08-08 18:25 UTC (permalink / raw)
  To: netdev; +Cc: linville, andrew, davem
In-Reply-To: <20180808182211.24921-3-f.fainelli@gmail.com>

On 08/08/2018 11:22 AM, Florian Fainelli wrote:
> Add a new character 'f' which can be used to configure an Ethernet
> controller to support Wake-on-LAN using filters programmed with the
> ethtool::rxnfc and the special action -2 (wake-up filter). This is
> useful in particular in the context of devices that must support wake-up
> on more complex patterns such as multicast DNS packets.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  ethtool.8.in | 4 +++-
>  ethtool.c    | 5 +++++
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/ethtool.8.in b/ethtool.8.in
> index 0a366aa536ae..97c7330fd373 100644
> --- a/ethtool.8.in
> +++ b/ethtool.8.in
> @@ -58,7 +58,7 @@
>  .\"
>  .\"	\(*WO - wol flags
>  .\"
> -.ds WO \fBp\fP|\fBu\fP|\fBm\fP|\fBb\fP|\fBa\fP|\fBg\fP|\fBs\fP|\fBd\fP...
> +.ds WO \fBp\fP|\fBu\fP|\fBm\fP|\fBb\fP|\fBa\fP|\fBg\fP|\fBs\fP|\fBf|\fBd\fP...
>  .\"
>  .\"	\(*FL - flow type values
>  .\"
> @@ -679,6 +679,7 @@ b	Wake on broadcast messages
>  a	Wake on ARP
>  g	Wake on MagicPacket\[tm]
>  s	Enable SecureOn\[tm] password for MagicPacket\[tm]
> +f	Wake on filter(s)
>  d	T{
>  Disable (wake on nothing).  This option clears all previous options.
>  T}
> @@ -870,6 +871,7 @@ Specifies the Rx queue to send packets to, or some other action.
>  nokeep;
>  lB	l.
>  -1	Drop the matched flow
> +-2	Use the matched flow as a Wake-on-LAN filter

Humm, this hunk belongs in patch #3, I will re-submit that series after
you give me some feedback. Thanks!
-- 
Florian

^ permalink raw reply

* Re: [PATCH net-next v6 11/11] net: sched: change action API to use array of pointers to actions
From: Cong Wang @ 2018-08-08 18:29 UTC (permalink / raw)
  To: Vlad Buslov
  Cc: Linux Kernel Network Developers, David Miller, Jamal Hadi Salim,
	Jiri Pirko, Alexei Starovoitov, Daniel Borkmann,
	Yevgeny Kliteynik
In-Reply-To: <vbfr2j92ii3.fsf@reg-r-vrt-018-180.mtr.labs.mlnx>

On Wed, Aug 8, 2018 at 4:41 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>
>
> On Tue 07 Aug 2018 at 23:26, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> > On Thu, Jul 5, 2018 at 7:24 AM Vlad Buslov <vladbu@mellanox.com> wrote:
> >>         attr_size = tcf_action_full_attrs_size(attr_size);
> >>
> >>         if (event == RTM_GETACTION)
> >> -               ret = tcf_get_notify(net, portid, n, &actions, event, extack);
> >> +               ret = tcf_get_notify(net, portid, n, actions, event, extack);
> >>         else { /* delete */
> >> -               ret = tcf_del_notify(net, n, &actions, portid, attr_size, extack);
> >> +               ret = tcf_del_notify(net, n, actions, &acts_deleted, portid,
> >> +                                    attr_size, extack);
> >>                 if (ret)
> >>                         goto err;
> >>                 return ret;
> >>         }
> >>  err:
> >> -       tcf_action_put_lst(&actions);
> >> +       tcf_action_put_many(&actions[acts_deleted]);
> >>         return ret;
> >
> > How does this even work?
> >
> > You save an index in 'acts_deleted', but you pass &actions[acts_deleted]
> > to tcf_action_put_many(), which seems you want to start from
> > where it fails, but inside tcf_action_put_many() it starts from 0
> > to TCA_ACT_MAX_PRIO, out-of-bound access at least?
>
> Actions array is declared to be TCA_ACT_MAX_PRIO+1 in size, and


Declaration doesn't matter at all, functions see it as a pure pointer
once you pass it as an argument.


> initialized to NULL pointers. In loop inside tcf_action_put_many() there
> are two checks: One is that index is less than TCA_ACT_MAX_PRIO and
> another one that pointer is not NULL. In this case I rely on extra NULL
> pointer at the end of actions array to prevent out-of-bound access.

True, but you pass &actions[acts_deleted] as the start of the array,
so inside it would be:

&actions[acts_deleted][0]...&actions[acts_deleted][MAX_PRIO]

So, the overall of the result is:

actions[acts_deleted]...actions[acts_deleted + MAX_PRIO]

You have out-of-bound access when acts_deleted > 1.

And if acts_deleted == MAX_PRIO-1, then you don't have any
NULL pointer to rely on.

^ permalink raw reply

* we give editing
From: Julian Jones @ 2018-08-08 11:28 UTC (permalink / raw)
  To: netdev

You have needs for image editing for our studio?
Our studio does different image editing for e-commerce photos, jewelry
images, and portrait mages.

It includes cutting out and clipping path and the others, like retouching
etc.

Drop us a photo, testing will be provided.

Thanks,
Julian

^ permalink raw reply

* [PATCH] net: phy: sftp: print debug message with text, not numbers
From: Andrew Lunn @ 2018-08-08 18:54 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Russell King, Florian Fainelli, Andrew Lunn

Convert the state numbers, device state, etc from numbers to strings
when printing debug messages.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/sfp.c | 76 ++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 72 insertions(+), 4 deletions(-)

diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
index 5661226cf75b..4637d980310e 100644
--- a/drivers/net/phy/sfp.c
+++ b/drivers/net/phy/sfp.c
@@ -60,6 +60,69 @@ enum {
 	SFP_S_TX_DISABLE,
 };
 
+static const char  * const mod_state_strings[] = {
+	[SFP_MOD_EMPTY] = "empty",
+	[SFP_MOD_PROBE] = "probe",
+	[SFP_MOD_HPOWER] = "hpower",
+	[SFP_MOD_PRESENT] = "present",
+	[SFP_MOD_ERROR] = "error",
+};
+
+static const char *mod_state_to_str(unsigned short mod_state)
+{
+	if (mod_state >= ARRAY_SIZE(mod_state_strings))
+		return "Unknown module state";
+	return mod_state_strings[mod_state];
+}
+
+static const char * const dev_state_strings[] = {
+	[SFP_DEV_DOWN] = "down",
+	[SFP_DEV_UP] = "up",
+};
+
+static const char *dev_state_to_str(unsigned short dev_state)
+{
+	if (dev_state >= ARRAY_SIZE(dev_state_strings))
+		return "Unknown device state";
+	return dev_state_strings[dev_state];
+}
+
+static const char * const event_strings[] = {
+	[SFP_E_INSERT] = "insert",
+	[SFP_E_REMOVE] = "remove",
+	[SFP_E_DEV_DOWN] = "dev_down",
+	[SFP_E_DEV_UP] = "dev_up",
+	[SFP_E_TX_FAULT] = "tx_fault",
+	[SFP_E_TX_CLEAR] = "tx_clear",
+	[SFP_E_LOS_HIGH] = "los_high",
+	[SFP_E_LOS_LOW] = "los_low",
+	[SFP_E_TIMEOUT] = "timeout",
+};
+
+static const char *event_to_str(unsigned short event)
+{
+	if (event >= ARRAY_SIZE(event_strings))
+		return "Unknown event";
+	return event_strings[event];
+}
+
+static const char * const sm_state_strings[] = {
+	[SFP_S_DOWN] = "down",
+	[SFP_S_INIT] = "init",
+	[SFP_S_WAIT_LOS] = "wait_los",
+	[SFP_S_LINK_UP] = "link_up",
+	[SFP_S_TX_FAULT] = "tx_fault",
+	[SFP_S_REINIT] = "reinit",
+	[SFP_S_TX_DISABLE] = "rx_disable",
+};
+
+static const char *sm_state_to_str(unsigned short sm_state)
+{
+	if (sm_state >= ARRAY_SIZE(sm_state_strings))
+		return "Unknown state";
+	return sm_state_strings[sm_state];
+}
+
 static const char *gpio_of_names[] = {
 	"mod-def0",
 	"los",
@@ -1388,8 +1451,11 @@ static void sfp_sm_event(struct sfp *sfp, unsigned int event)
 {
 	mutex_lock(&sfp->sm_mutex);
 
-	dev_dbg(sfp->dev, "SM: enter %u:%u:%u event %u\n",
-		sfp->sm_mod_state, sfp->sm_dev_state, sfp->sm_state, event);
+	dev_dbg(sfp->dev, "SM: enter %s:%s:%s event %s\n",
+		mod_state_to_str(sfp->sm_mod_state),
+		dev_state_to_str(sfp->sm_dev_state),
+		sm_state_to_str(sfp->sm_state),
+		event_to_str(event));
 
 	/* This state machine tracks the insert/remove state of
 	 * the module, and handles probing the on-board EEPROM.
@@ -1520,8 +1586,10 @@ static void sfp_sm_event(struct sfp *sfp, unsigned int event)
 		break;
 	}
 
-	dev_dbg(sfp->dev, "SM: exit %u:%u:%u\n",
-		sfp->sm_mod_state, sfp->sm_dev_state, sfp->sm_state);
+	dev_dbg(sfp->dev, "SM: exit %s:%s:%s\n",
+		mod_state_to_str(sfp->sm_mod_state),
+		dev_state_to_str(sfp->sm_dev_state),
+		sm_state_to_str(sfp->sm_state));
 
 	mutex_unlock(&sfp->sm_mutex);
 }
-- 
2.18.0

^ permalink raw reply related

* [PATCH] dsa: slave: eee: Allow ports to use phylink
From: Andrew Lunn @ 2018-08-08 18:56 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Florian Fainelli, Andrew Lunn

For a port to be able to use EEE, both the MAC and the PHY must
support EEE. A phy can be provided by both a phydev or phylink. Verify
at least one of these exist, not just phydev.

Fixes: aab9c4067d23 ("net: dsa: Plug in PHYLINK support")
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 net/dsa/slave.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 80b71bab3e59..973e99aa4107 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -640,7 +640,7 @@ static int dsa_slave_set_eee(struct net_device *dev, struct ethtool_eee *e)
 	int ret;
 
 	/* Port's PHY and MAC both need to be EEE capable */
-	if (!dev->phydev)
+	if (!dev->phydev && !dp->pl)
 		return -ENODEV;
 
 	if (!ds->ops->set_mac_eee)
@@ -660,7 +660,7 @@ static int dsa_slave_get_eee(struct net_device *dev, struct ethtool_eee *e)
 	int ret;
 
 	/* Port's PHY and MAC both need to be EEE capable */
-	if (!dev->phydev)
+	if (!dev->phydev && !dp->pl)
 		return -ENODEV;
 
 	if (!ds->ops->get_mac_eee)
-- 
2.18.0

^ permalink raw reply related

* Re: [PATCH] dsa: slave: eee: Allow ports to use phylink
From: Florian Fainelli @ 2018-08-08 19:00 UTC (permalink / raw)
  To: Andrew Lunn, David Miller; +Cc: netdev
In-Reply-To: <1533754600-1753-1-git-send-email-andrew@lunn.ch>

On 08/08/2018 11:56 AM, Andrew Lunn wrote:
> For a port to be able to use EEE, both the MAC and the PHY must
> support EEE. A phy can be provided by both a phydev or phylink. Verify
> at least one of these exist, not just phydev.
> 
> Fixes: aab9c4067d23 ("net: dsa: Plug in PHYLINK support")
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ permalink raw reply

* Re: [PATCH] net: phy: sftp: print debug message with text, not numbers
From: Florian Fainelli @ 2018-08-08 19:02 UTC (permalink / raw)
  To: Andrew Lunn, David Miller; +Cc: netdev, Russell King
In-Reply-To: <1533754452-1633-1-git-send-email-andrew@lunn.ch>

On 08/08/2018 11:54 AM, Andrew Lunn wrote:
> Convert the state numbers, device state, etc from numbers to strings
> when printing debug messages.

I had a similar patch locally that I used for initial troubleshooting,
which I might even have shared with Russell at some point, though now I
can't remember if he was okay or not with the idea. This is much more
readable IMHO so:

Acked-by: Florian Fainelli <f.fainelli@gmail.com>

> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  drivers/net/phy/sfp.c | 76 ++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 72 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
> index 5661226cf75b..4637d980310e 100644
> --- a/drivers/net/phy/sfp.c
> +++ b/drivers/net/phy/sfp.c
> @@ -60,6 +60,69 @@ enum {
>  	SFP_S_TX_DISABLE,
>  };
>  
> +static const char  * const mod_state_strings[] = {
> +	[SFP_MOD_EMPTY] = "empty",
> +	[SFP_MOD_PROBE] = "probe",
> +	[SFP_MOD_HPOWER] = "hpower",
> +	[SFP_MOD_PRESENT] = "present",
> +	[SFP_MOD_ERROR] = "error",
> +};
> +
> +static const char *mod_state_to_str(unsigned short mod_state)
> +{
> +	if (mod_state >= ARRAY_SIZE(mod_state_strings))
> +		return "Unknown module state";
> +	return mod_state_strings[mod_state];
> +}
> +
> +static const char * const dev_state_strings[] = {
> +	[SFP_DEV_DOWN] = "down",
> +	[SFP_DEV_UP] = "up",
> +};
> +
> +static const char *dev_state_to_str(unsigned short dev_state)
> +{
> +	if (dev_state >= ARRAY_SIZE(dev_state_strings))
> +		return "Unknown device state";
> +	return dev_state_strings[dev_state];
> +}
> +
> +static const char * const event_strings[] = {
> +	[SFP_E_INSERT] = "insert",
> +	[SFP_E_REMOVE] = "remove",
> +	[SFP_E_DEV_DOWN] = "dev_down",
> +	[SFP_E_DEV_UP] = "dev_up",
> +	[SFP_E_TX_FAULT] = "tx_fault",
> +	[SFP_E_TX_CLEAR] = "tx_clear",
> +	[SFP_E_LOS_HIGH] = "los_high",
> +	[SFP_E_LOS_LOW] = "los_low",
> +	[SFP_E_TIMEOUT] = "timeout",
> +};
> +
> +static const char *event_to_str(unsigned short event)
> +{
> +	if (event >= ARRAY_SIZE(event_strings))
> +		return "Unknown event";
> +	return event_strings[event];
> +}
> +
> +static const char * const sm_state_strings[] = {
> +	[SFP_S_DOWN] = "down",
> +	[SFP_S_INIT] = "init",
> +	[SFP_S_WAIT_LOS] = "wait_los",
> +	[SFP_S_LINK_UP] = "link_up",
> +	[SFP_S_TX_FAULT] = "tx_fault",
> +	[SFP_S_REINIT] = "reinit",
> +	[SFP_S_TX_DISABLE] = "rx_disable",
> +};
> +
> +static const char *sm_state_to_str(unsigned short sm_state)
> +{
> +	if (sm_state >= ARRAY_SIZE(sm_state_strings))
> +		return "Unknown state";
> +	return sm_state_strings[sm_state];
> +}
> +
>  static const char *gpio_of_names[] = {
>  	"mod-def0",
>  	"los",
> @@ -1388,8 +1451,11 @@ static void sfp_sm_event(struct sfp *sfp, unsigned int event)
>  {
>  	mutex_lock(&sfp->sm_mutex);
>  
> -	dev_dbg(sfp->dev, "SM: enter %u:%u:%u event %u\n",
> -		sfp->sm_mod_state, sfp->sm_dev_state, sfp->sm_state, event);
> +	dev_dbg(sfp->dev, "SM: enter %s:%s:%s event %s\n",
> +		mod_state_to_str(sfp->sm_mod_state),
> +		dev_state_to_str(sfp->sm_dev_state),
> +		sm_state_to_str(sfp->sm_state),
> +		event_to_str(event));
>  
>  	/* This state machine tracks the insert/remove state of
>  	 * the module, and handles probing the on-board EEPROM.
> @@ -1520,8 +1586,10 @@ static void sfp_sm_event(struct sfp *sfp, unsigned int event)
>  		break;
>  	}
>  
> -	dev_dbg(sfp->dev, "SM: exit %u:%u:%u\n",
> -		sfp->sm_mod_state, sfp->sm_dev_state, sfp->sm_state);
> +	dev_dbg(sfp->dev, "SM: exit %s:%s:%s\n",
> +		mod_state_to_str(sfp->sm_mod_state),
> +		dev_state_to_str(sfp->sm_dev_state),
> +		sm_state_to_str(sfp->sm_state));
>  
>  	mutex_unlock(&sfp->sm_mutex);
>  }
> 


-- 
Florian

^ permalink raw reply

* Re: [PATCH bpf 0/3] Couple of sockmap fixes
From: Alexei Starovoitov @ 2018-08-08 19:13 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: netdev, ast, john.fastabend
In-Reply-To: <20180808172315.4710-1-daniel@iogearbox.net>

On Wed, Aug 08, 2018 at 07:23:12PM +0200, Daniel Borkmann wrote:
> Two sockmap fixes in bpf_tcp_sendmsg(), and one fix for the
> sockmap kernel selftest. Thanks!

test_sockmap jumped from 10 to 47 seconds :(
but applied anyway.
Thanks

^ permalink raw reply

* Re: [PATCH net-next,v4] net/tls: Calculate nsg for zerocopy path without skb_cow_data.
From: David Miller @ 2018-08-08 19:14 UTC (permalink / raw)
  To: doronrk; +Cc: davejwatson, vakul.garg, borisp, aviadye, netdev
In-Reply-To: <20180807180939.3872231-1-doronrk@fb.com>

From: Doron Roberts-Kedes <doronrk@fb.com>
Date: Tue, 7 Aug 2018 11:09:39 -0700

> +static int __skb_nsg(struct sk_buff *skb, int offset, int len,
> +		     unsigned int recursion_level)
> +{
> +	int start = skb_headlen(skb);
> +	int i, copy = start - offset;
> +	struct sk_buff *frag_iter;
> +	int elt = 0;
> +
> +	if (unlikely(recursion_level >= 24))
> +		return -EMSGSIZE;

This recursion is kinda crazy.

Even skb_cow_data() doesn't recurse like this (of course because it copies
into linear buffers).

There has to be a way to simplify this.  Fragment lists are such a rarely
used SKB geometry, and few if any devices support it for transmission
(so the fraglist will get undone at transmit time anyways).

> +	// We need one extra for ctx->rx_aad_ciphertext

Please do not use C++ style comments in code.

Thanks.

^ permalink raw reply

* possible deadlock in team_vlan_rx_add_vid
From: syzbot @ 2018-08-08 21:39 UTC (permalink / raw)
  To: davem, jiri, linux-kernel, netdev, syzkaller-bugs

Hello,

syzbot found the following crash on:

HEAD commit:    0b5b1f9a78b5 Merge tag 'for-linus' of git://git.kernel.org..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=1514a09c400000
kernel config:  https://syzkaller.appspot.com/x/.config?x=2dc0cd7c2eefb46f
dashboard link: https://syzkaller.appspot.com/bug?extid=bd051aba086537515cdb
compiler:       gcc (GCC) 8.0.1 20180413 (experimental)
syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=16b10a54400000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+bd051aba086537515cdb@syzkaller.appspotmail.com

8021q: adding VLAN 0 to HW filter on device team0
8021q: adding VLAN 0 to HW filter on device team0
8021q: adding VLAN 0 to HW filter on device team0

============================================
WARNING: possible recursive locking detected
4.18.0-rc7+ #176 Not tainted
--------------------------------------------
syz-executor4/6391 is trying to acquire lock:
(____ptrval____) (&team->lock){+.+.}, at: team_vlan_rx_add_vid+0x3b/0x1e0  
drivers/net/team/team.c:1868

but task is already holding lock:
(____ptrval____) (&team->lock){+.+.}, at: team_add_slave+0xdb/0x1c30  
drivers/net/team/team.c:1947

other info that might help us debug this:
  Possible unsafe locking scenario:

        CPU0
        ----
   lock(&team->lock);
   lock(&team->lock);

  *** DEADLOCK ***

  May be due to missing lock nesting notation

2 locks held by syz-executor4/6391:
  #0: (____ptrval____) (rtnl_mutex){+.+.}, at: rtnl_lock  
net/core/rtnetlink.c:77 [inline]
  #0: (____ptrval____) (rtnl_mutex){+.+.}, at: rtnetlink_rcv_msg+0x412/0xc30  
net/core/rtnetlink.c:4662
  #1: (____ptrval____) (&team->lock){+.+.}, at: team_add_slave+0xdb/0x1c30  
drivers/net/team/team.c:1947

stack backtrace:
CPU: 1 PID: 6391 Comm: syz-executor4 Not tainted 4.18.0-rc7+ #176
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Call Trace:
  __dump_stack lib/dump_stack.c:77 [inline]
  dump_stack+0x1c9/0x2b4 lib/dump_stack.c:113
  print_deadlock_bug kernel/locking/lockdep.c:1765 [inline]
  check_deadlock kernel/locking/lockdep.c:1809 [inline]
  validate_chain kernel/locking/lockdep.c:2405 [inline]
  __lock_acquire.cold.64+0x1fb/0x486 kernel/locking/lockdep.c:3435
  lock_acquire+0x1e4/0x540 kernel/locking/lockdep.c:3924
  __mutex_lock_common kernel/locking/mutex.c:757 [inline]
  __mutex_lock+0x176/0x1820 kernel/locking/mutex.c:894
  mutex_lock_nested+0x16/0x20 kernel/locking/mutex.c:909
  team_vlan_rx_add_vid+0x3b/0x1e0 drivers/net/team/team.c:1868
  vlan_add_rx_filter_info+0x14a/0x1d0 net/8021q/vlan_core.c:210
  __vlan_vid_add net/8021q/vlan_core.c:278 [inline]
  vlan_vid_add+0x63e/0x9d0 net/8021q/vlan_core.c:308
  vlan_device_event.cold.12+0x2a/0x2f net/8021q/vlan.c:381
  notifier_call_chain+0x180/0x390 kernel/notifier.c:93
  __raw_notifier_call_chain kernel/notifier.c:394 [inline]
  raw_notifier_call_chain+0x2d/0x40 kernel/notifier.c:401
  call_netdevice_notifiers_info+0x3f/0x90 net/core/dev.c:1735
  call_netdevice_notifiers net/core/dev.c:1753 [inline]
  dev_open+0x173/0x1b0 net/core/dev.c:1433
  team_port_add drivers/net/team/team.c:1219 [inline]
  team_add_slave+0xa8b/0x1c30 drivers/net/team/team.c:1948
  do_set_master+0x1c9/0x220 net/core/rtnetlink.c:2248
  do_setlink+0xba4/0x3e10 net/core/rtnetlink.c:2382
  rtnl_setlink+0x2a9/0x400 net/core/rtnetlink.c:2636
  rtnetlink_rcv_msg+0x46e/0xc30 net/core/rtnetlink.c:4665
  netlink_rcv_skb+0x172/0x440 net/netlink/af_netlink.c:2455
  rtnetlink_rcv+0x1c/0x20 net/core/rtnetlink.c:4683
  netlink_unicast_kernel net/netlink/af_netlink.c:1317 [inline]
  netlink_unicast+0x5a0/0x760 net/netlink/af_netlink.c:1343
  netlink_sendmsg+0xa18/0xfd0 net/netlink/af_netlink.c:1908
  sock_sendmsg_nosec net/socket.c:642 [inline]
  sock_sendmsg+0xd5/0x120 net/socket.c:652
  ___sys_sendmsg+0x7fd/0x930 net/socket.c:2126
  __sys_sendmsg+0x11d/0x290 net/socket.c:2164
  __do_sys_sendmsg net/socket.c:2173 [inline]
  __se_sys_sendmsg net/socket.c:2171 [inline]
  __x64_sys_sendmsg+0x78/0xb0 net/socket.c:2171
  do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
  entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x456b29
Code: fd b4 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7  
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff  
ff 0f 83 cb b4 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007f9706bf8c78 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 00007f9706bf96d4 RCX: 0000000000456b29
RDX: 0000000000000000 RSI: 0000000020000240 RDI: 0000000000000004
RBP: 00000000009300a0 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00000000ffffffff
R13: 00000000004d3548 R14: 00000000004c8227 R15: 0000000000000000


---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with  
syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches

^ permalink raw reply

* Re: [PATCH] net: phy: sftp: print debug message with text, not numbers
From: Andrew Lunn @ 2018-08-08 19:18 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: David Miller, netdev, Russell King
In-Reply-To: <6f042c52-fdc9-c90c-82ab-2a7b38c04583@gmail.com>

On Wed, Aug 08, 2018 at 12:02:56PM -0700, Florian Fainelli wrote:
> On 08/08/2018 11:54 AM, Andrew Lunn wrote:
> > Convert the state numbers, device state, etc from numbers to strings
> > when printing debug messages.
> 
> I had a similar patch locally that I used for initial troubleshooting,
> which I might even have shared with Russell at some point, though now I
> can't remember if he was okay or not with the idea. This is much more
> readable IMHO so:

Yes, I should of included an example:

[   87.359862] sfp sff3: SM: enter present:down:down event dev_up
[   87.359875] sfp sff3: tx disable 1 -> 0
[   87.359900] sfp sff3: SM: exit present:up:init
[   87.360099] IPv6: ADDRCONF(NETDEV_UP): sff3: link is not ready
[   87.661662] sfp sff3: SM: enter present:up:init event timeout
[   87.661697] sfp sff3: SM: exit present:up:link_up
[   87.661827] mv88e6085 0.2:00 sff3: Link is Up - 1Gbps/Full - flow control off

    Andrew

^ permalink raw reply

* [PATCH bpf 0/3] Couple of sockmap fixes
From: Daniel Borkmann @ 2018-08-08 19:33 UTC (permalink / raw)
  To: Alexei Starovoitov; +Cc: netdev, ast, john.fastabend
In-Reply-To: <20180808191259.r5akjmsv5mdzmpje@ast-mbp>

On 08/08/2018 09:13 PM, Alexei Starovoitov wrote:
> On Wed, Aug 08, 2018 at 07:23:12PM +0200, Daniel Borkmann wrote:
>> Two sockmap fixes in bpf_tcp_sendmsg(), and one fix for the
>> sockmap kernel selftest. Thanks!
> 
> test_sockmap jumped from 10 to 47 seconds :(

Agree, it's unfortunate, but otherwise the tests keep failing since the fix
in patch 1 does propagate errors now. So while test_sockmap before that fix
was succeeding for me, it correctly exposed the EPIPE now due to the timeout,
so it's a minimal workaround (for the time being).

> but applied anyway.
> Thanks

Thanks!

^ permalink raw reply

* Re: [PATCH net-next] decnet: fix using plain integer as NULL warning
From: Kees Cook @ 2018-08-08 22:03 UTC (permalink / raw)
  To: YueHaibing
  Cc: David S. Miller, LKML, Network Development, Eric Dumazet,
	linux-decnet-user
In-Reply-To: <20180808115932.12944-1-yuehaibing@huawei.com>

On Wed, Aug 8, 2018 at 4:59 AM, YueHaibing <yuehaibing@huawei.com> wrote:
> Fixes the following sparse warning:
> net/decnet/dn_route.c:407:30: warning: Using plain integer as NULL pointer
> net/decnet/dn_route.c:1923:22: warning: Using plain integer as NULL pointer
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>

Reviewed-by: Kees Cook <keescook@chromium.org>

-Kees

-- 
Kees Cook
Pixel Security

^ permalink raw reply

* Re: [PATCH bpf-next 12/13] docs: net: Fix various minor typos
From: Tobin C. Harding @ 2018-08-08 22:21 UTC (permalink / raw)
  To: Markus Heiser
  Cc: Daniel Borkmann, Jonathan Corbet, Alexei Starovoitov,
	David S. Miller, linux-doc, netdev, linux-kernel
In-Reply-To: <1cfb4fb3406621c1ffd98e8072f2cc38812107d8.camel@darmarit.de>

On Wed, Aug 08, 2018 at 06:26:50PM +0200, Markus Heiser wrote:
> 
> 
> Am Mittwoch, den 08.08.2018, 15:45 +0200 schrieb Daniel Borkmann:
> > On 08/08/2018 03:23 PM, Jonathan Corbet wrote:
> > > On Wed, 8 Aug 2018 11:42:48 +1000
> > > "Tobin C. Harding" <me@tobin.cc> wrote:
> > > 
> > > > Thanks for doing such a careful review that you noticed this.  I'm
> > > > working on this more ATM and I've moved the document to use double
> > > > spaces between _all_ full stops.  Currently the document uses mostly
> > > > single spaces but there are some sections with double space.  The
> > > > internet tells me this is a 'style' issue not a rule.  And I've seen
> > > > single and double spacing in tree and do not know if one is favoured.
> > > > 
> > > > Do you care? 
> > > 
> > > I'm not Daniel, but let me just say that, for docs in general, I'm
> > > absolutely uninterested in patches adjusting the number of spaces after
> > > periods.  It really doesn't matter one way or the other, and I don't think
> > > we benefit from that kind of churn.
> > 
> > Yep, agree.
> 
> FWIW: even if it is not a patch worth, 'fill-paragraph' within emacs benefit
> from.

That's why I originally started using double spaces :)

thanks,
Tobin.

^ permalink raw reply

* Re: [PATCH 24/33] net/mlx5e: Mark expected switch fall-throughs
From: Saeed Mahameed @ 2018-08-08 22:26 UTC (permalink / raw)
  To: Gustavo A. R. Silva
  Cc: David S. Miller, Linux Netdev List, linux-kernel, Saeed Mahameed,
	Leon Romanovsky, RDMA mailing list
In-Reply-To: <3c523c58a8e3f3d240d4ff9363d672fb977c9d93.1533675546.git.gustavo@embeddedor.com>

On Tue, Aug 7, 2018 at 4:25 PM, Gustavo A. R. Silva
<gustavo@embeddedor.com> wrote:
> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> where we are expecting to fall through.
>
> Addresses-Coverity-ID: 114808 ("Missing break in switch")
> Addresses-Coverity-ID: 114802 ("Missing break in switch")
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>

Acked-by: Saeed Mahameed <saeedm@mellanox.com>

> ---
>  drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c b/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c
> index 1881468..ad6d471 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c
> @@ -91,9 +91,11 @@ bool mlx5e_xdp_handle(struct mlx5e_rq *rq, struct mlx5e_dma_info *di,
>                 return true;
>         default:
>                 bpf_warn_invalid_xdp_action(act);
> +               /* fall through */
>         case XDP_ABORTED:
>  xdp_abort:
>                 trace_xdp_exception(rq->netdev, prog, act);
> +               /* fall through */
>         case XDP_DROP:
>                 rq->stats->xdp_drop++;
>                 return true;
> --
> 2.7.4
>

^ permalink raw reply

* Re: [PATCH net-next] ieee802154: hwsim: fix rcu handling
From: Stefan Schmidt @ 2018-08-08 20:05 UTC (permalink / raw)
  To: Alexander Aring, netdev; +Cc: linux-wpan, kernel
In-Reply-To: <20180807233249.28572-1-aring@mojatatu.com>

Hello David.

I want to apologize how the submission of the ieee802154_hwsim driver
worked out. The sparse warning finally fixed by this commit came due to
an outdated sparse used on our side while kbuild was up to date. I fixed
this now and sparse has no warnings for the ieee802154_hwsim driver
anymore with this patch applied.

The other two fixes from Wei Yongjun are problems I should have spotted
during code review, but did not. No excuse, I just overlooked them.

I would, once again, ask you to apply these three directly to net-next.
I you would prefer for me to pick them up and send you a pull request
this time just let me know and I will do so tomorrow.

On 08/08/2018 01:32 AM, Alexander Aring wrote:
> This patch adds missing rcu_assign_pointer()/rcu_dereference() to used rcu
> pointers. There was already a previous commit c5d99d2b35da ("ieee802154:
> hwsim: fix rcu address annotation"), but there was more which was
> pointed out on my side by using newest sparse version.
> 
> Cc: Stefan Schmidt <stefan@datenfreihafen.org>
> Fixes: f25da51fdc38 ("ieee802154: hwsim: add replacement for fakelb")
> Signed-off-by: Alexander Aring <aring@mojatatu.com>
> ---
> After I installed finally the newest sparse version I found more what kbuild
> was pointed out.
> 
> I hope I did it right, not sure if I really need rcu functionality in these
> case because protection by mutex but I make sparse silent.
> At some places the resource isn't a shared resource at this moment.
> 
> Anyway in my case I want to use this driver to create easily multi-hop
> scenarios, if somebody report things (or I hit it) which smells like some
> memory in this area - I will try and help to fix it.
> 
> Newest sparse version from git is happy now and I hope kbuild bot as well.
> 
> I tested this patch with RPOVE_RCU enabled and tried to update these settings
> while heavy loading in rcu protected xmit hotpath is occured, without any
> issues so far.
> 
> Sorry again.
> 
>  drivers/net/ieee802154/mac802154_hwsim.c | 24 +++++++++++++++++++-----
>  1 file changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/ieee802154/mac802154_hwsim.c b/drivers/net/ieee802154/mac802154_hwsim.c
> index f4e92054f7df..53f39435321e 100644
> --- a/drivers/net/ieee802154/mac802154_hwsim.c
> +++ b/drivers/net/ieee802154/mac802154_hwsim.c
> @@ -22,6 +22,7 @@
>  #include <linux/module.h>
>  #include <linux/timer.h>
>  #include <linux/platform_device.h>
> +#include <linux/rtnetlink.h>
>  #include <linux/netdevice.h>
>  #include <linux/device.h>
>  #include <linux/spinlock.h>
> @@ -110,7 +111,7 @@ static int hwsim_hw_channel(struct ieee802154_hw *hw, u8 page, u8 channel)
>  	pib->page = page;
>  	pib->channel = channel;
>  
> -	pib_old = phy->pib;
> +	pib_old = rtnl_dereference(phy->pib);
>  	rcu_assign_pointer(phy->pib, pib);
>  	kfree_rcu(pib_old, rcu);
>  	return 0;
> @@ -406,7 +407,7 @@ static struct hwsim_edge *hwsim_alloc_edge(struct hwsim_phy *endpoint, u8 lqi)
>  	}
>  
>  	einfo->lqi = 0xff;
> -	e->info = einfo;
> +	rcu_assign_pointer(e->info, einfo);
>  	e->endpoint = endpoint;
>  
>  	return e;
> @@ -414,7 +415,13 @@ static struct hwsim_edge *hwsim_alloc_edge(struct hwsim_phy *endpoint, u8 lqi)
>  
>  static void hwsim_free_edge(struct hwsim_edge *e)
>  {
> -	kfree_rcu(e->info, rcu);
> +	struct hwsim_edge_info *einfo;
> +
> +	rcu_read_lock();
> +	einfo = rcu_dereference(e->info);
> +	rcu_read_unlock();
> +
> +	kfree_rcu(einfo, rcu);
>  	kfree_rcu(e, rcu);
>  }
>  
> @@ -796,7 +803,7 @@ static int hwsim_add_one(struct genl_info *info, struct device *dev,
>  		goto err_pib;
>  	}
>  
> -	phy->pib = pib;
> +	rcu_assign_pointer(phy->pib, pib);
>  	phy->idx = idx;
>  	INIT_LIST_HEAD(&phy->edges);
>  
> @@ -829,10 +836,17 @@ static int hwsim_add_one(struct genl_info *info, struct device *dev,
>  
>  static void hwsim_del(struct hwsim_phy *phy)
>  {
> +	struct hwsim_pib *pib;
> +
>  	hwsim_edge_unsubscribe_me(phy);
>  
>  	list_del(&phy->list);
> -	kfree_rcu(phy->pib, rcu);
> +
> +	rcu_read_lock();
> +	pib = rcu_dereference(phy->pib);
> +	rcu_read_unlock();
> +
> +	kfree_rcu(pib, rcu);
>  
>  	ieee802154_unregister_hw(phy->hw);
>  	ieee802154_free_hw(phy->hw);
> 

Signed-off-by: Stefan Schmidt <stefan@datenfreihafen.org>

regards
Stefan Schmidt

^ permalink raw reply

* Re: [PATCH net-next] ieee802154: hwsim: fix copy-paste error in hwsim_set_edge_lqi()
From: Stefan Schmidt @ 2018-08-08 20:06 UTC (permalink / raw)
  To: Wei Yongjun, Alexander Aring; +Cc: linux-wpan, netdev, kernel-janitors
In-Reply-To: <1533696226-26445-1-git-send-email-weiyongjun1@huawei.com>

Hello Wei.

On 08/08/2018 04:43 AM, Wei Yongjun wrote:
> The return value from kzalloc() is not checked correctly. The
> test is done against a wrong variable. This patch fix it.
> 
> Fixes: f25da51fdc38 ("ieee802154: hwsim: add replacement for fakelb")
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
> ---
>  drivers/net/ieee802154/mac802154_hwsim.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ieee802154/mac802154_hwsim.c b/drivers/net/ieee802154/mac802154_hwsim.c
> index 1982308..04f4100 100644
> --- a/drivers/net/ieee802154/mac802154_hwsim.c
> +++ b/drivers/net/ieee802154/mac802154_hwsim.c
> @@ -564,7 +564,7 @@ static int hwsim_set_edge_lqi(struct sk_buff *msg, struct genl_info *info)
>  	}
>  
>  	einfo = kzalloc(sizeof(*einfo), GFP_KERNEL);
> -	if (!info) {
> +	if (!einfo) {
>  		mutex_unlock(&hwsim_phys_lock);
>  		return -ENOMEM;
>  	}
> 

Thanks a lot for finding and fixing these two bugs!

Signed-off-by: Stefan Schmidt <stefan@datenfreihafen.org>

regards
Stefan Schmidt

^ permalink raw reply

* Re: [PATCH net-next] ieee802154: hwsim: fix missing unlock on error in hwsim_add_one()
From: Stefan Schmidt @ 2018-08-08 20:06 UTC (permalink / raw)
  To: Wei Yongjun, Alexander Aring; +Cc: linux-wpan, netdev, kernel-janitors
In-Reply-To: <1533697839-85602-1-git-send-email-weiyongjun1@huawei.com>

Hello Wei.

On 08/08/2018 05:10 AM, Wei Yongjun wrote:
> Add the missing unlock before return from function hwsim_add_one()
> in the error handling case.
> 
> Fixes: f25da51fdc38 ("ieee802154: hwsim: add replacement for fakelb")
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
> ---
>  drivers/net/ieee802154/mac802154_hwsim.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ieee802154/mac802154_hwsim.c b/drivers/net/ieee802154/mac802154_hwsim.c
> index f4e9205..44d398c 100644
> --- a/drivers/net/ieee802154/mac802154_hwsim.c
> +++ b/drivers/net/ieee802154/mac802154_hwsim.c
> @@ -810,8 +810,10 @@ static int hwsim_add_one(struct genl_info *info, struct device *dev,
>  	mutex_lock(&hwsim_phys_lock);
>  	if (init) {
>  		err = hwsim_subscribe_all_others(phy);
> -		if (err < 0)
> +		if (err < 0) {
> +			mutex_unlock(&hwsim_phys_lock);
>  			goto err_reg;
> +		}
>  	}
>  	list_add_tail(&phy->list, &hwsim_phys);
>  	mutex_unlock(&hwsim_phys_lock);
> 

Signed-off-by: Stefan Schmidt <stefan@datenfreihafen.org>

regards
Stefan Schmidt

^ permalink raw reply

* Re: [PATCH] 9p: fix NULL pointer dereferences
From: Dominique Martinet @ 2018-08-08 22:41 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: Tomas Bortoli, David Miller, v9fs-developer, netdev, LKML,
	syzkaller
In-Reply-To: <CACT4Y+Z+Q__-cMaQ-85H38Rs5AeFGitXoE8-Bv4pNsvat+G+xQ@mail.gmail.com>

Dmitry Vyukov wrote on Wed, Aug 08, 2018:
> > If you want to factor it in, v9fs_mount does not know which transport is
> > used, but p9_client_create does know - although the functions/structs
> > are all static so you need to check clnt->trans_mod->name with strcmp
> > and I'm honestly not sure that's better than checking in each function..
> >
> > But, as usual I'm happy as long as it works, so pick your poison :)
> 
> So let's proceed with checking in each transport function?

Yes, he's done it a while ago, just didn't se the in-reply-to field to
keep part of the thread but the Reported-by was here:
http://lkml.kernel.org/r/20180727110558.5479-1-tomasbortoli@gmail.com

This is in linux-next right now as 631263a1b23c71

-- 
Dominique

^ permalink raw reply

* [PATCH net-next] rds: avoid lock hierarchy violation between m_rs_lock and rs_recv_lock
From: Sowmini Varadhan @ 2018-08-08 20:57 UTC (permalink / raw)
  To: netdev, sowmini.varadhan
  Cc: davem, rds-devel, sowmini.varadhan, santosh.shilimkar

The following deadlock, reported by syzbot, can occur if CPU0 is in
rds_send_remove_from_sock() while CPU1 is in rds_clear_recv_queue()

       CPU0                    CPU1
       ----                    ----
  lock(&(&rm->m_rs_lock)->rlock);
                               lock(&rs->rs_recv_lock);
                               lock(&(&rm->m_rs_lock)->rlock);
  lock(&rs->rs_recv_lock);

The deadlock should be avoided by moving the messages from the
rs_recv_queue into a tmp_list in rds_clear_recv_queue() under
the rs_recv_lock, and then dropping the refcnt on the messages
in the tmp_list (potentially resulting in rds_message_purge())
after dropping the rs_recv_lock.

The same lock hierarchy violation also exists in rds_still_queued()
and should be avoided in a similar manner

Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
Reported-by: syzbot+52140d69ac6dc6b927a9@syzkaller.appspotmail.com
---
 net/rds/recv.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/net/rds/recv.c b/net/rds/recv.c
index 504cd6b..1cf7072 100644
--- a/net/rds/recv.c
+++ b/net/rds/recv.c
@@ -429,6 +429,7 @@ static int rds_still_queued(struct rds_sock *rs, struct rds_incoming *inc,
 	struct sock *sk = rds_rs_to_sk(rs);
 	int ret = 0;
 	unsigned long flags;
+	bool drop_ref = false;
 
 	write_lock_irqsave(&rs->rs_recv_lock, flags);
 	if (!list_empty(&inc->i_item)) {
@@ -439,11 +440,13 @@ static int rds_still_queued(struct rds_sock *rs, struct rds_incoming *inc,
 					      -be32_to_cpu(inc->i_hdr.h_len),
 					      inc->i_hdr.h_dport);
 			list_del_init(&inc->i_item);
-			rds_inc_put(inc);
+			drop_ref = true;
 		}
 	}
 	write_unlock_irqrestore(&rs->rs_recv_lock, flags);
 
+	if (drop_ref)
+		rds_inc_put(inc);
 	rdsdebug("inc %p rs %p still %d dropped %d\n", inc, rs, ret, drop);
 	return ret;
 }
@@ -751,16 +754,20 @@ void rds_clear_recv_queue(struct rds_sock *rs)
 	struct sock *sk = rds_rs_to_sk(rs);
 	struct rds_incoming *inc, *tmp;
 	unsigned long flags;
+	LIST_HEAD(tmp_list);
 
 	write_lock_irqsave(&rs->rs_recv_lock, flags);
 	list_for_each_entry_safe(inc, tmp, &rs->rs_recv_queue, i_item) {
 		rds_recv_rcvbuf_delta(rs, sk, inc->i_conn->c_lcong,
 				      -be32_to_cpu(inc->i_hdr.h_len),
 				      inc->i_hdr.h_dport);
+		list_move_tail(&inc->i_item, &tmp_list);
+	}
+	write_unlock_irqrestore(&rs->rs_recv_lock, flags);
+	list_for_each_entry_safe(inc, tmp, &tmp_list, i_item) {
 		list_del_init(&inc->i_item);
 		rds_inc_put(inc);
 	}
-	write_unlock_irqrestore(&rs->rs_recv_lock, flags);
 }
 
 /*
-- 
1.7.1

^ permalink raw reply related

* [net PATCH 0/3] Fix two teardown bugs for BPF maps cpumap and devmap
From: Jesper Dangaard Brouer @ 2018-08-08 21:00 UTC (permalink / raw)
  To: netdev, Jesper Dangaard Brouer
  Cc: Daniel Borkmann, Alexei Starovoitov, jhsiao

Removing entries from cpumap and devmap, goes through a number of
syncronization steps to make sure no new xdp_frames can be enqueued.
But there is a small chance, that xdp_frames remains which have not
been flushed/processed yet.  Flushing these during teardown, happens
from RCU context and not as usual under RX NAPI context.

The optimization introduced in commt 389ab7f01af9 ("xdp: introduce
xdp_return_frame_rx_napi"), missed that the flush operation can also
be called from RCU context.  Thus, we cannot always use the
xdp_return_frame_rx_napi call, which take advantage of the protection
provided by XDP RX running under NAPI protection.

The samples/bpf xdp_redirect_cpu have a --stress-mode, that is
adjusted to easier reproduce (verified by Red Hat QA).

---

Jesper Dangaard Brouer (3):
      xdp: fix bug in cpumap teardown code path
      samples/bpf: xdp_redirect_cpu adjustment to reproduce teardown race easier
      xdp: fix bug in devmap teardown code path


 kernel/bpf/cpumap.c                 |   15 +++++++++------
 kernel/bpf/devmap.c                 |   14 +++++++++-----
 samples/bpf/xdp_redirect_cpu_kern.c |    2 +-
 samples/bpf/xdp_redirect_cpu_user.c |    4 ++--
 4 files changed, 21 insertions(+), 14 deletions(-)

^ permalink raw reply

* [net PATCH 1/3] xdp: fix bug in cpumap teardown code path
From: Jesper Dangaard Brouer @ 2018-08-08 21:00 UTC (permalink / raw)
  To: netdev, Jesper Dangaard Brouer
  Cc: Daniel Borkmann, Alexei Starovoitov, jhsiao
In-Reply-To: <153376197849.14272.8201612461878004477.stgit@firesoul>

When removing a cpumap entry, a number of syncronization steps happen.
Eventually the teardown code __cpu_map_entry_free is invoked from/via
call_rcu.

The teardown code __cpu_map_entry_free() flushes remaining xdp_frames,
by invoking bq_flush_to_queue, which calls xdp_return_frame_rx_napi().
The issues is that the teardown code is not running in the RX NAPI
code path.  Thus, it is not allowed to invoke the NAPI variant of
xdp_return_frame.

This bug was found and triggered by using the --stress-mode option to
the samples/bpf program xdp_redirect_cpu.  It is hard to trigger,
because the ptr_ring have to be full and cpumap bulk queue max
contains 8 packets, and a remote CPU is racing to empty the ptr_ring
queue.

Fixes: 389ab7f01af9 ("xdp: introduce xdp_return_frame_rx_napi")
Tested-by: Jean-Tsung Hsiao <jhsiao@redhat.com>
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
 kernel/bpf/cpumap.c |   15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/kernel/bpf/cpumap.c b/kernel/bpf/cpumap.c
index e0918d180f08..46f5f29605d4 100644
--- a/kernel/bpf/cpumap.c
+++ b/kernel/bpf/cpumap.c
@@ -69,7 +69,7 @@ struct bpf_cpu_map {
 };
 
 static int bq_flush_to_queue(struct bpf_cpu_map_entry *rcpu,
-			     struct xdp_bulk_queue *bq);
+			     struct xdp_bulk_queue *bq, bool in_napi_ctx);
 
 static u64 cpu_map_bitmap_size(const union bpf_attr *attr)
 {
@@ -375,7 +375,7 @@ static void __cpu_map_entry_free(struct rcu_head *rcu)
 		struct xdp_bulk_queue *bq = per_cpu_ptr(rcpu->bulkq, cpu);
 
 		/* No concurrent bq_enqueue can run at this point */
-		bq_flush_to_queue(rcpu, bq);
+		bq_flush_to_queue(rcpu, bq, false);
 	}
 	free_percpu(rcpu->bulkq);
 	/* Cannot kthread_stop() here, last put free rcpu resources */
@@ -558,7 +558,7 @@ const struct bpf_map_ops cpu_map_ops = {
 };
 
 static int bq_flush_to_queue(struct bpf_cpu_map_entry *rcpu,
-			     struct xdp_bulk_queue *bq)
+			     struct xdp_bulk_queue *bq, bool in_napi_ctx)
 {
 	unsigned int processed = 0, drops = 0;
 	const int to_cpu = rcpu->cpu;
@@ -578,7 +578,10 @@ static int bq_flush_to_queue(struct bpf_cpu_map_entry *rcpu,
 		err = __ptr_ring_produce(q, xdpf);
 		if (err) {
 			drops++;
-			xdp_return_frame_rx_napi(xdpf);
+			if (likely(in_napi_ctx))
+				xdp_return_frame_rx_napi(xdpf);
+			else
+				xdp_return_frame(xdpf);
 		}
 		processed++;
 	}
@@ -598,7 +601,7 @@ static int bq_enqueue(struct bpf_cpu_map_entry *rcpu, struct xdp_frame *xdpf)
 	struct xdp_bulk_queue *bq = this_cpu_ptr(rcpu->bulkq);
 
 	if (unlikely(bq->count == CPU_MAP_BULK_SIZE))
-		bq_flush_to_queue(rcpu, bq);
+		bq_flush_to_queue(rcpu, bq, true);
 
 	/* Notice, xdp_buff/page MUST be queued here, long enough for
 	 * driver to code invoking us to finished, due to driver
@@ -661,7 +664,7 @@ void __cpu_map_flush(struct bpf_map *map)
 
 		/* Flush all frames in bulkq to real queue */
 		bq = this_cpu_ptr(rcpu->bulkq);
-		bq_flush_to_queue(rcpu, bq);
+		bq_flush_to_queue(rcpu, bq, true);
 
 		/* If already running, costs spin_lock_irqsave + smb_mb */
 		wake_up_process(rcpu->kthread);

^ permalink raw reply related

* [net PATCH 2/3] samples/bpf: xdp_redirect_cpu adjustment to reproduce teardown race easier
From: Jesper Dangaard Brouer @ 2018-08-08 21:00 UTC (permalink / raw)
  To: netdev, Jesper Dangaard Brouer
  Cc: Daniel Borkmann, Alexei Starovoitov, jhsiao
In-Reply-To: <153376197849.14272.8201612461878004477.stgit@firesoul>

The teardown race in cpumap is really hard to reproduce.  These changes
makes it easier to reproduce, for QA.

The --stress-mode now have a case of a very small queue size of 8, that helps
to trigger teardown flush to encounter a full queue, which results in calling
xdp_return_frame API, in a non-NAPI protect context.

Also increase MAX_CPUS, as my QA department have larger machines than me.

Tested-by: Jean-Tsung Hsiao <jhsiao@redhat.com>
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
 samples/bpf/xdp_redirect_cpu_kern.c |    2 +-
 samples/bpf/xdp_redirect_cpu_user.c |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/samples/bpf/xdp_redirect_cpu_kern.c b/samples/bpf/xdp_redirect_cpu_kern.c
index 8cb703671b04..0cc3d71057f0 100644
--- a/samples/bpf/xdp_redirect_cpu_kern.c
+++ b/samples/bpf/xdp_redirect_cpu_kern.c
@@ -14,7 +14,7 @@
 #include <uapi/linux/bpf.h>
 #include "bpf_helpers.h"
 
-#define MAX_CPUS 12 /* WARNING - sync with _user.c */
+#define MAX_CPUS 64 /* WARNING - sync with _user.c */
 
 /* Special map type that can XDP_REDIRECT frames to another CPU */
 struct bpf_map_def SEC("maps") cpu_map = {
diff --git a/samples/bpf/xdp_redirect_cpu_user.c b/samples/bpf/xdp_redirect_cpu_user.c
index f6efaefd485b..4b4d78fffe30 100644
--- a/samples/bpf/xdp_redirect_cpu_user.c
+++ b/samples/bpf/xdp_redirect_cpu_user.c
@@ -19,7 +19,7 @@ static const char *__doc__ =
 #include <arpa/inet.h>
 #include <linux/if_link.h>
 
-#define MAX_CPUS 12 /* WARNING - sync with _kern.c */
+#define MAX_CPUS 64 /* WARNING - sync with _kern.c */
 
 /* How many xdp_progs are defined in _kern.c */
 #define MAX_PROG 5
@@ -527,7 +527,7 @@ static void stress_cpumap(void)
 	 * procedure.
 	 */
 	create_cpu_entry(1,  1024, 0, false);
-	create_cpu_entry(1,   128, 0, false);
+	create_cpu_entry(1,     8, 0, false);
 	create_cpu_entry(1, 16000, 0, false);
 }
 

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox