Netdev List
 help / color / mirror / Atom feed
* [PATCH iproute2 net 6/8] tc/pedit: p_eth: ETH header editor
From: Amir Vadai @ 2017-04-23 12:53 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, Or Gerlitz, Jamal Hadi Salim, Amir Vadai
In-Reply-To: <20170423125356.1298-1-amir@vadai.me>

For example, forward tcp traffic to veth0 and set
destination mac address to 11:22:33:44:55:66 :
$ tc filter add dev enp0s9 protocol ip parent ffff: \
    flower \
      ip_proto tcp \
    action pedit ex munge \
      eth dst set 11:22:33:44:55:66 \
    action mirred egress \
      redirect dev veth0

Signed-off-by: Amir Vadai <amir@vadai.me>
---
 man/man8/tc-pedit.8 | 24 ++++++++++++++++++
 tc/Makefile         |  1 +
 tc/m_pedit.c        | 46 ++++++++++++++++++++++++++++++++++
 tc/m_pedit.h        |  1 +
 tc/p_eth.c          | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 144 insertions(+)
 create mode 100644 tc/p_eth.c

diff --git a/man/man8/tc-pedit.8 b/man/man8/tc-pedit.8
index c98d95cb0021..8febdfe23f6e 100644
--- a/man/man8/tc-pedit.8
+++ b/man/man8/tc-pedit.8
@@ -27,12 +27,18 @@ pedit - generic packet editor action
 
 .ti -8
 .IR EXTENDED_LAYERED_OP " := { "
+.BI eth " ETHHDR_FIELD"
+|
 .BI ip " IPHDR_FIELD"
 |
 .BI ip " EX_IPHDR_FIELD"
 .RI } " CMD_SPEC"
 
 .ti -8
+.IR ETHHDR_FIELD " := { "
+.BR src " | " dst " | " type " }"
+
+.ti -8
 .IR IPHDR_FIELD " := { "
 .BR src " | " dst " | " tos " | " dsfield " | " ihl " | " protocol " |"
 .BR precedence " | " nofrag " | " firstfrag " | " ce " | " df " }"
@@ -103,6 +109,21 @@ and right-shifted by
 before adding it to
 .IR OFFSET .
 .TP
+.BI eth " ETHHDR_FIELD"
+Change an ETH header field. The supported keywords for
+.I ETHHDR_FIELD
+are:
+.RS
+.TP
+.B src
+.TQ
+.B dst
+Source or destination MAC address in the standard format: XX:XX:XX:XX:XX:XX
+.TP
+.B type
+Ether-type in numeric value
+.RE
+.TP
 .BI ip " IPHDR_FIELD"
 Change an IPv4 header field. The supported keywords for
 .I IPHDR_FIELD
@@ -269,6 +290,9 @@ tc filter add dev eth0 parent ffff: u32 \\
 tc filter add dev eth0 parent ffff: u32 \\
 	match ip sport 22 0xffff \\
 	action pedit ex munge ip dst set 192.168.1.199
+tc filter add dev eth0 parent ffff: u32 \\
+	match ip sport 22 0xffff \\
+	action pedit ex munge eth dst set 11:22:33:44:55:66
 .EE
 .RE
 .SH SEE ALSO
diff --git a/tc/Makefile b/tc/Makefile
index 3f7fc939e194..446a11391ad7 100644
--- a/tc/Makefile
+++ b/tc/Makefile
@@ -54,6 +54,7 @@ TCMODULES += m_tunnel_key.o
 TCMODULES += m_sample.o
 TCMODULES += p_ip.o
 TCMODULES += p_icmp.o
+TCMODULES += p_eth.o
 TCMODULES += p_tcp.o
 TCMODULES += p_udp.o
 TCMODULES += em_nbyte.o
diff --git a/tc/m_pedit.c b/tc/m_pedit.c
index d982c91a2585..0be42343ac88 100644
--- a/tc/m_pedit.c
+++ b/tc/m_pedit.c
@@ -28,6 +28,7 @@
 #include "utils.h"
 #include "tc_util.h"
 #include "m_pedit.h"
+#include "rt_names.h"
 
 static struct m_pedit_util *pedit_list;
 static int pedit_debug;
@@ -223,6 +224,38 @@ int pack_key8(__u32 retain, struct m_pedit_sel *sel, struct m_pedit_key *tkey)
 	return pack_key(sel, tkey);
 }
 
+static int pack_mac(struct m_pedit_sel *sel, struct m_pedit_key *tkey,
+		    __u8 *mac)
+{
+	int ret = 0;
+
+	if (!(tkey->off & 0x3)) {
+		tkey->mask = 0;
+		tkey->val = ntohl(*((__u32 *)mac));
+		ret |= pack_key32(~0, sel, tkey);
+
+		tkey->off += 4;
+		tkey->mask = 0;
+		tkey->val = ntohs(*((__u16 *)&mac[4]));
+		ret |= pack_key16(~0, sel, tkey);
+	} else if (!(tkey->off & 0x1)) {
+		tkey->mask = 0;
+		tkey->val = ntohs(*((__u16 *)mac));
+		ret |= pack_key16(~0, sel, tkey);
+
+		tkey->off += 4;
+		tkey->mask = 0;
+		tkey->val = ntohl(*((__u32 *)(mac + 2)));
+		ret |= pack_key32(~0, sel, tkey);
+	} else {
+		fprintf(stderr,
+			"pack_mac: mac offsets must begin in 32bit or 16bit boundaries\n");
+		return -1;
+	}
+
+	return ret;
+}
+
 int parse_val(int *argc_p, char ***argv_p, __u32 *val, int type)
 {
 	int argc = *argc_p;
@@ -250,6 +283,14 @@ int parse_val(int *argc_p, char ***argv_p, __u32 *val, int type)
 	if (type == TIPV6)
 		return -1; /* not implemented yet */
 
+	if (type == TMAC) {
+#define MAC_ALEN 6
+		int ret = ll_addr_a2n((char *)val, MAC_ALEN, *argv);
+
+		if (ret == MAC_ALEN)
+			return 0;
+	}
+
 	return -1;
 }
 
@@ -310,6 +351,11 @@ int parse_cmd(int *argc_p, char ***argv_p, __u32 len, int type, __u32 retain,
 		argv++;
 	}
 
+	if (type == TMAC) {
+		res = pack_mac(sel, tkey, (__u8 *)val);
+		goto done;
+	}
+
 	tkey->val = *v;
 	tkey->mask = *m;
 
diff --git a/tc/m_pedit.h b/tc/m_pedit.h
index e2897b0c9808..ecfb6add0df5 100644
--- a/tc/m_pedit.h
+++ b/tc/m_pedit.h
@@ -32,6 +32,7 @@
 #define TIPV6 2
 #define TINT 3
 #define TU32 4
+#define TMAC 5
 
 #define RU32 0xFFFFFFFF
 #define RU16 0xFFFF
diff --git a/tc/p_eth.c b/tc/p_eth.c
new file mode 100644
index 000000000000..ad3e28f80eb6
--- /dev/null
+++ b/tc/p_eth.c
@@ -0,0 +1,72 @@
+/*
+ * m_pedit_eth.c	packet editor: ETH header
+ *
+ *		This program is free software; you can distribute 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:  Amir Vadai (amir@vadai.me)
+ *
+ */
+
+#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"
+#include "m_pedit.h"
+
+static int
+parse_eth(int *argc_p, char ***argv_p,
+	  struct m_pedit_sel *sel, struct m_pedit_key *tkey)
+{
+	int res = -1;
+	int argc = *argc_p;
+	char **argv = *argv_p;
+
+	if (argc < 2)
+		return -1;
+
+	tkey->htype = TCA_PEDIT_KEY_EX_HDR_TYPE_ETH;
+
+	if (strcmp(*argv, "type") == 0) {
+		NEXT_ARG();
+		tkey->off = 12;
+		res = parse_cmd(&argc, &argv, 2, TU32, RU16, sel, tkey);
+		goto done;
+	}
+
+	if (strcmp(*argv, "dst") == 0) {
+		NEXT_ARG();
+		tkey->off = 0;
+		res = parse_cmd(&argc, &argv, 6, TMAC, RU32, sel, tkey);
+		goto done;
+	}
+
+	if (strcmp(*argv, "src") == 0) {
+		NEXT_ARG();
+		tkey->off = 6;
+		res = parse_cmd(&argc, &argv, 6, TMAC, RU32, sel, tkey);
+		goto done;
+	}
+
+	return -1;
+
+done:
+	*argc_p = argc;
+	*argv_p = argv;
+	return res;
+}
+
+struct m_pedit_util p_pedit_eth = {
+	NULL,
+	"eth",
+	parse_eth,
+};
-- 
2.12.0

^ permalink raw reply related

* [PATCH iproute2 net 8/8] tc/pedit: p_udp: introduce pedit udp support
From: Amir Vadai @ 2017-04-23 12:53 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, Or Gerlitz, Jamal Hadi Salim, Amir Vadai
In-Reply-To: <20170423125356.1298-1-amir@vadai.me>

From: Or Gerlitz <ogerlitz@mellanox.com>

For example, forward udp traffic destined to port 999 to veth0 and set
tcp port to 888:
$ tc filter add dev enp0s9 protocol ip parent ffff: \
    flower \
      ip_proto udp \
      dst_port 999 \
    action pedit ex munge \
      udp dport set 888 \
    action mirred egress \
      redirect dev veth0

Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
Signed-off-by: Amir Vadai <amir@vadai.me>
---
 man/man8/tc-pedit.8 | 18 ++++++++++++++++++
 tc/p_udp.c          | 27 +++++++++++++++++++++++++++
 2 files changed, 45 insertions(+)

diff --git a/man/man8/tc-pedit.8 b/man/man8/tc-pedit.8
index ad1929592660..7f482eafc6c7 100644
--- a/man/man8/tc-pedit.8
+++ b/man/man8/tc-pedit.8
@@ -34,6 +34,8 @@ pedit - generic packet editor action
 .BI ip " EX_IPHDR_FIELD"
 |
 .BI tcp " TCPHDR_FIELD"
+|
+.BI udp " UDPHDR_FIELD"
 .RI } " CMD_SPEC"
 
 .ti -8
@@ -58,6 +60,10 @@ pedit - generic packet editor action
 .BR sport " | " dport " | " flags " }"
 
 .ti -8
+.IR UDPHDR_FIELD " := { "
+.BR sport " | " dport " }"
+
+.ti -8
 .IR CMD_SPEC " := {"
 .BR clear " | " invert " | " set
 .IR VAL " | "
@@ -219,6 +225,18 @@ Source or destination TCP port number, a 16-bit value.
 .B flags
 .RE
 .TP
+.BI udp " UDPHDR_FIELD"
+The supported keywords for
+.I UDPHDR_FIELD
+are:
+.RS
+.TP
+.B sport
+.TQ
+.B dport
+Source or destination TCP port number, a 16-bit value.
+.RE
+.TP
 .B clear
 Clear the addressed data (i.e., set it to zero).
 .TP
diff --git a/tc/p_udp.c b/tc/p_udp.c
index 3a86ba382391..a56a1b519254 100644
--- a/tc/p_udp.c
+++ b/tc/p_udp.c
@@ -28,6 +28,33 @@ parse_udp(int *argc_p, char ***argv_p,
 	  struct m_pedit_sel *sel, struct m_pedit_key *tkey)
 {
 	int res = -1;
+	int argc = *argc_p;
+	char **argv = *argv_p;
+
+	if (argc < 2)
+		return -1;
+
+	tkey->htype = TCA_PEDIT_KEY_EX_HDR_TYPE_UDP;
+
+	if (strcmp(*argv, "sport") == 0) {
+		NEXT_ARG();
+		tkey->off = 0;
+		res = parse_cmd(&argc, &argv, 2, TU32, RU16, sel, tkey);
+		goto done;
+	}
+
+	if (strcmp(*argv, "dport") == 0) {
+		NEXT_ARG();
+		tkey->off = 2;
+		res = parse_cmd(&argc, &argv, 2, TU32, RU16, sel, tkey);
+		goto done;
+	}
+
+	return -1;
+
+done:
+	*argc_p = argc;
+	*argv_p = argv;
 	return res;
 }
 
-- 
2.12.0

^ permalink raw reply related

* [PATCH iproute2 net 5/8] tc/pedit: Support fields bigger than 32 bits
From: Amir Vadai @ 2017-04-23 12:53 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, Or Gerlitz, Jamal Hadi Salim, Amir Vadai
In-Reply-To: <20170423125356.1298-1-amir@vadai.me>

Make parse_val() accept fields up to 128 bits long, this should be
enough for current use cases and involves a minimal change to code.

Signed-off-by: Amir Vadai <amir@vadai.me>
---
 tc/m_pedit.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/tc/m_pedit.c b/tc/m_pedit.c
index 7af074a5a97c..d982c91a2585 100644
--- a/tc/m_pedit.c
+++ b/tc/m_pedit.c
@@ -256,7 +256,10 @@ int parse_val(int *argc_p, char ***argv_p, __u32 *val, int type)
 int parse_cmd(int *argc_p, char ***argv_p, __u32 len, int type, __u32 retain,
 	      struct m_pedit_sel *sel, struct m_pedit_key *tkey)
 {
-	__u32 mask = 0, val = 0;
+	__u32 mask[4] = { 0 };
+	__u32 val[4] = { 0 };
+	__u32 *m = &mask[0];
+	__u32 *v = &val[0];
 	__u32 o = 0xFF;
 	int res = -1;
 	int argc = *argc_p;
@@ -275,7 +278,7 @@ int parse_cmd(int *argc_p, char ***argv_p, __u32 len, int type, __u32 retain,
 		o = 0xFFFFFFFF;
 
 	if (matches(*argv, "invert") == 0) {
-		val = mask = o;
+		*v = *m = o;
 	} else if (matches(*argv, "set") == 0 ||
 		   matches(*argv, "add") == 0) {
 		if (matches(*argv, "add") == 0)
@@ -287,7 +290,7 @@ int parse_cmd(int *argc_p, char ***argv_p, __u32 len, int type, __u32 retain,
 		}
 
 		NEXT_ARG();
-		if (parse_val(&argc, &argv, &val, type))
+		if (parse_val(&argc, &argv, val, type))
 			return -1;
 	} else if (matches(*argv, "preserve") == 0) {
 		retain = 0;
@@ -307,8 +310,8 @@ int parse_cmd(int *argc_p, char ***argv_p, __u32 len, int type, __u32 retain,
 		argv++;
 	}
 
-	tkey->val = val;
-	tkey->mask = mask;
+	tkey->val = *v;
+	tkey->mask = *m;
 
 	if (type == TIPV4)
 		tkey->val = ntohl(tkey->val);
-- 
2.12.0

^ permalink raw reply related

* [PATCH iproute2 net 7/8] tc/pedit: p_tcp: introduce pedit tcp support
From: Amir Vadai @ 2017-04-23 12:53 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, Or Gerlitz, Jamal Hadi Salim, Amir Vadai
In-Reply-To: <20170423125356.1298-1-amir@vadai.me>

For example, forward tcp traffic destined to port 80 to veth0 and set
tcp port to 8080:
$ tc filter add dev enp0s9 protocol ip parent ffff: \
    flower \
      ip_proto tcp \
      dst_port 80 \
    action pedit ex munge \
      tcp dport set 8080 \
    action mirred egress \
      redirect dev veth0

Signed-off-by: Amir Vadai <amir@vadai.me>
---
 man/man8/tc-pedit.8 | 23 +++++++++++++++++++++++
 tc/p_tcp.c          | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+)

diff --git a/man/man8/tc-pedit.8 b/man/man8/tc-pedit.8
index 8febdfe23f6e..ad1929592660 100644
--- a/man/man8/tc-pedit.8
+++ b/man/man8/tc-pedit.8
@@ -32,6 +32,8 @@ pedit - generic packet editor action
 .BI ip " IPHDR_FIELD"
 |
 .BI ip " EX_IPHDR_FIELD"
+|
+.BI tcp " TCPHDR_FIELD"
 .RI } " CMD_SPEC"
 
 .ti -8
@@ -52,6 +54,10 @@ pedit - generic packet editor action
 .BR ttl " }"
 
 .ti -8
+.IR TCPHDR_FIELD " := { "
+.BR sport " | " dport " | " flags " }"
+
+.ti -8
 .IR CMD_SPEC " := {"
 .BR clear " | " invert " | " set
 .IR VAL " | "
@@ -199,6 +205,20 @@ are:
 .B ttl
 .RE
 .TP
+.BI tcp " TCPHDR_FIELD"
+The supported keywords for
+.I TCPHDR_FIELD
+are:
+.RS
+.TP
+.B sport
+.TQ
+.B dport
+Source or destination TCP port number, a 16-bit value.
+.TP
+.B flags
+.RE
+.TP
 .B clear
 Clear the addressed data (i.e., set it to zero).
 .TP
@@ -293,6 +313,9 @@ tc filter add dev eth0 parent ffff: u32 \\
 tc filter add dev eth0 parent ffff: u32 \\
 	match ip sport 22 0xffff \\
 	action pedit ex munge eth dst set 11:22:33:44:55:66
+tc filter add dev eth0 parent ffff: u32 \\
+	match ip dport 23 0xffff \\
+	action pedit ex munge tcp dport set 22
 .EE
 .RE
 .SH SEE ALSO
diff --git a/tc/p_tcp.c b/tc/p_tcp.c
index 53ee9842160b..cf14574c9c3e 100644
--- a/tc/p_tcp.c
+++ b/tc/p_tcp.c
@@ -28,6 +28,43 @@ parse_tcp(int *argc_p, char ***argv_p,
 	  struct m_pedit_sel *sel, struct m_pedit_key *tkey)
 {
 	int res = -1;
+	int argc = *argc_p;
+	char **argv = *argv_p;
+
+	if (argc < 2)
+		return -1;
+
+	if (!sel->extended)
+		return -1;
+
+	tkey->htype = TCA_PEDIT_KEY_EX_HDR_TYPE_TCP;
+
+	if (strcmp(*argv, "sport") == 0) {
+		NEXT_ARG();
+		tkey->off = 0;
+		res = parse_cmd(&argc, &argv, 2, TU32, RU16, sel, tkey);
+		goto done;
+	}
+
+	if (strcmp(*argv, "dport") == 0) {
+		NEXT_ARG();
+		tkey->off = 2;
+		res = parse_cmd(&argc, &argv, 2, TU32, RU16, sel, tkey);
+		goto done;
+	}
+
+	if (strcmp(*argv, "flags") == 0) {
+		NEXT_ARG();
+		tkey->off = 13;
+		res = parse_cmd(&argc, &argv, 1, TU32, RU8, sel, tkey);
+		goto done;
+	}
+
+	return -1;
+
+done:
+	*argc_p = argc;
+	*argv_p = argv;
 	return res;
 }
 struct m_pedit_util p_pedit_tcp = {
-- 
2.12.0

^ permalink raw reply related

* [PATCH 1/1] libertas: check return value of alloc_workqueue
From: Pan Bian @ 2017-04-23 13:19 UTC (permalink / raw)
  To: Kalle Valo, Bhaktipriya Shridhar, Tejun Heo
  Cc: libertas-dev, linux-wireless, netdev, linux-kernel, Pan Bian

From: Pan Bian <bianpan2016@163.com>

Function alloc_workqueue() will return a NULL pointer if there is no
enough memory, and its return value should be validated before using.
However, in function if_spi_probe(), its return value is not checked.
This may result in a NULL dereference bug. This patch fixes the bug.

Signed-off-by: Pan Bian <bianpan2016@163.com>
---
 drivers/net/wireless/marvell/libertas/if_spi.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/wireless/marvell/libertas/if_spi.c b/drivers/net/wireless/marvell/libertas/if_spi.c
index c3a53cd..7b4955c 100644
--- a/drivers/net/wireless/marvell/libertas/if_spi.c
+++ b/drivers/net/wireless/marvell/libertas/if_spi.c
@@ -1181,6 +1181,10 @@ static int if_spi_probe(struct spi_device *spi)
 
 	/* Initialize interrupt handling stuff. */
 	card->workqueue = alloc_workqueue("libertas_spi", WQ_MEM_RECLAIM, 0);
+	if (!card->workqueue) {
+		err = -ENOMEM;
+		goto remove_card;
+	}
 	INIT_WORK(&card->packet_work, if_spi_host_to_card_worker);
 	INIT_WORK(&card->resume_work, if_spi_resume_worker);
 
@@ -1209,6 +1213,7 @@ static int if_spi_probe(struct spi_device *spi)
 	free_irq(spi->irq, card);
 terminate_workqueue:
 	destroy_workqueue(card->workqueue);
+remove_card:
 	lbs_remove_card(priv); /* will call free_netdev */
 free_card:
 	free_if_spi_card(card);
-- 
1.9.1

^ permalink raw reply related

* [PATCH 1/1] cfg80211: add return value validation
From: Pan Bian @ 2017-04-23 13:27 UTC (permalink / raw)
  To: Jussi Kivilinna, Kalle Valo,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Pan Bian

From: Pan Bian <bianpan2016-9Onoh4P/yGk@public.gmane.org>

Function create_singlethread_workqueue() will return a NULL pointer if
there is no enough memory, and its return value should be validated
before using. However, in function rndis_wlan_bind(), its return value
is not checked. This may cause NULL dereference bugs. This patch fixes
it.

Signed-off-by: Pan Bian <bianpan2016-9Onoh4P/yGk@public.gmane.org>
---
 drivers/net/wireless/rndis_wlan.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/wireless/rndis_wlan.c b/drivers/net/wireless/rndis_wlan.c
index 785334f..92a1bde 100644
--- a/drivers/net/wireless/rndis_wlan.c
+++ b/drivers/net/wireless/rndis_wlan.c
@@ -3427,6 +3427,10 @@ static int rndis_wlan_bind(struct usbnet *usbdev, struct usb_interface *intf)
 
 	/* because rndis_command() sleeps we need to use workqueue */
 	priv->workqueue = create_singlethread_workqueue("rndis_wlan");
+	if (!priv->workqueue) {
+		wiphy_free(wiphy);
+		return -ENOMEM;
+	}
 	INIT_WORK(&priv->work, rndis_wlan_worker);
 	INIT_DELAYED_WORK(&priv->dev_poller_work, rndis_device_poller);
 	INIT_DELAYED_WORK(&priv->scan_work, rndis_get_scan_results);
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH 1/1] tipc: check return value of nlmsg_new
From: Jon Maloy @ 2017-04-23 13:33 UTC (permalink / raw)
  To: Pan Bian, Ying Xue, David S. Miller
  Cc: netdev@vger.kernel.org, tipc-discussion@lists.sourceforge.net,
	linux-kernel@vger.kernel.org
In-Reply-To: <1492931359-25004-1-git-send-email-bianpan2016@163.com>

Acknowledged. Thank you for doing this job.

///jon


> -----Original Message-----
> From: Pan Bian [mailto:bianpan2016@163.com]
> Sent: Sunday, April 23, 2017 03:09 AM
> To: Jon Maloy <jon.maloy@ericsson.com>; Ying Xue
> <ying.xue@windriver.com>; David S. Miller <davem@davemloft.net>
> Cc: netdev@vger.kernel.org; tipc-discussion@lists.sourceforge.net; linux-
> kernel@vger.kernel.org; Pan Bian <bianpan2016@163.com>
> Subject: [PATCH 1/1] tipc: check return value of nlmsg_new
> 
> Function nlmsg_new() will return a NULL pointer if there is no enough
> memory, and its return value should be checked before it is used.
> However, in function tipc_nl_node_get_monitor(), the validation of the
> return value of function nlmsg_new() is missed. This patch fixes the bug.
> 
> Signed-off-by: Pan Bian <bianpan2016@163.com>
> ---
>  net/tipc/node.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/net/tipc/node.c b/net/tipc/node.c index 4512e83..568e48d 100644
> --- a/net/tipc/node.c
> +++ b/net/tipc/node.c
> @@ -2098,6 +2098,8 @@ int tipc_nl_node_get_monitor(struct sk_buff *skb,
> struct genl_info *info)
>  	int err;
> 
>  	msg.skb = nlmsg_new(NLMSG_GOODSIZE, GFP_KERNEL);
> +	if (!msg.skb)
> +		return -ENOMEM;
>  	msg.portid = info->snd_portid;
>  	msg.seq = info->snd_seq;
> 
> --
> 1.9.1
> 


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

^ permalink raw reply

* Re: [PATCH v2 net] net: ipv6: regenerate host route if moved to gc list
From: David Ahern @ 2017-04-23 14:08 UTC (permalink / raw)
  To: Martin KaFai Lau; +Cc: netdev, dvyukov, andreyknvl, mmanning
In-Reply-To: <20170423022801.t7lw3vuazo2ks6u4@kafai-mba.local>

On 4/22/17 8:28 PM, Martin KaFai Lau wrote:
>> The code path to fixup_permanent_addr is under RTNL, so the if check on
>> ifp->rt and rt6i_ref is ok -- neither can be changed since RTNL is held.
>>
>> Since ifp->rt can be accessed outside of RTNL, the spinlock is needed to
>> change its value.
> Got it. It is to protect the readers which are not under RTNL.
> Many thanks for pointing out what I was missing.  It all makes sense now.
> 
>> Arguably only 'ifp->rt = rt;' needs the spinlock.
> It still seems like the existing 'ifp->rt = rt;' needs protection
> anyway regardless of the rt regeneration change.  It would be nice to
> explain it in the commit log or even better separating it out
> into another patch.

I'll add a comment to the commit log when I send a v3 tomorrow morning.

^ permalink raw reply

* [PATCH net-next] bpf, doc: update list of architectures that do eBPF JIT
From: Alexei Starovoitov @ 2017-04-23 16:01 UTC (permalink / raw)
  To: David S . Miller; +Cc: Daniel Borkmann, netdev

update the list and remove 'in the future' statement,
since all still alive 64-bit architectures now do eBPF JIT.

Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
mips64 is the only 'still alive' 64-bit arch without eBPF JIT :)
---
 Documentation/networking/filter.txt | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/Documentation/networking/filter.txt b/Documentation/networking/filter.txt
index 683ada5ad81d..b69b205501de 100644
--- a/Documentation/networking/filter.txt
+++ b/Documentation/networking/filter.txt
@@ -595,10 +595,9 @@ got from bpf_prog_create(), and 'ctx' the given context (e.g.
 skb pointer). All constraints and restrictions from bpf_check_classic() apply
 before a conversion to the new layout is being done behind the scenes!
 
-Currently, the classic BPF format is being used for JITing on most of the
-architectures. x86-64, aarch64 and s390x perform JIT compilation from eBPF
-instruction set, however, future work will migrate other JIT compilers as well,
-so that they will profit from the very same benefits.
+Currently, the classic BPF format is being used for JITing on most 32-bit
+architectures, whereas x86-64, aarch64, s390x, powerpc64, sparc64 perform JIT
+compilation from eBPF instruction set.
 
 Some core changes of the new internal format:
 
-- 
2.9.3

^ permalink raw reply related

* Re: [PATCH iproute2 1/1] actions: Add support for user cookies
From: Stephen Hemminger @ 2017-04-23 16:11 UTC (permalink / raw)
  To: Jamal Hadi Salim; +Cc: netdev
In-Reply-To: <1492864583-20892-1-git-send-email-jhs@emojatatu.com>

On Sat, 22 Apr 2017 08:36:23 -0400
Jamal Hadi Salim <jhs@mojatatu.com> wrote:

> From: Jamal Hadi Salim <jhs@mojatatu.com>
> 
> Make use of 128b user cookies
> 
> Introduce optional 128-bit action cookie.
> Like all other cookie schemes in the networking world (eg in protocols
> like http or existing kernel fib protocol field, etc) the idea is to
> save user state that when retrieved serves as a correlator. The kernel
> _should not_ intepret it. The user can store whatever they wish in the
> 128 bits.
> 
> Sample exercise(showing variable length use of cookie)
> 
> .. create an accept action with cookie a1b2c3d4
> sudo $TC actions add action ok index 1 cookie a1b2c3d4
> 
> .. dump all gact actions..
> sudo $TC -s actions ls action gact
> 
>     action order 0: gact action pass
>      random type none pass val 0
>      index 1 ref 1 bind 0 installed 5 sec used 5 sec
>     Action statistics:
>     Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
>     backlog 0b 0p requeues 0
>     cookie a1b2c3d4
> 
> .. bind the accept action to a filter..
> sudo $TC filter add dev lo parent ffff: protocol ip prio 1 \
> u32 match ip dst 127.0.0.1/32 flowid 1:1 action gact index 1
> 
> ... send some traffic..
> $ ping 127.0.0.1 -c 3
> PING 127.0.0.1 (127.0.0.1) 56(84) bytes of data.
> 64 bytes from 127.0.0.1: icmp_seq=1 ttl=64 time=0.020 ms
> 64 bytes from 127.0.0.1: icmp_seq=2 ttl=64 time=0.027 ms
> 64 bytes from 127.0.0.1: icmp_seq=3 ttl=64 time=0.038 ms
> 
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>

Applied. Please update man page as well.

^ permalink raw reply

* Re: tools/testing/selftests/bpf/Makefile
From: Alexei Starovoitov @ 2017-04-23 16:13 UTC (permalink / raw)
  To: David Miller; +Cc: daniel, netdev
In-Reply-To: <20170422.154501.1876167225428231606.davem@davemloft.net>

On Sat, Apr 22, 2017 at 03:45:01PM -0400, David Miller wrote:
> 
> Alexei, that unconditional -D__x86_64__ isn't going to work.  It in
> fact makes the build break on sparc because the types.h asm headers
> explicitly check for things like __sparc__ && __arch64__ etc.

yeah. it was a quick workaround for the following error:
In file included from net-next/tools/testing/selftests/bpf/test_xdp.c:8:
In file included from /usr/include/string.h:25:
In file included from /usr/include/features.h:399:
/usr/include/gnu/stubs.h:7:11: fatal error: 'gnu/stubs-32.h' file not found

the bpf progs don't do any x86 specific things.

> In every
> 	arch/${ARCH}/Makefile
> extract out the "-DXXX" stuff from CHECKFLAGS into a new Makefile

that should work. Probably there is even simpler way.

^ permalink raw reply

* Re: [PATCH iproute2] gre6: fix copy/paste bugs in GREv6 attribute manipulation
From: Stephen Hemminger @ 2017-04-23 16:15 UTC (permalink / raw)
  To: Craig Gallek; +Cc: netdev
In-Reply-To: <20170421181425.166260-1-kraigatgoog@gmail.com>

On Fri, 21 Apr 2017 14:14:25 -0400
Craig Gallek <kraigatgoog@gmail.com> wrote:

> From: Craig Gallek <kraig@google.com>
> 
> Fixes: af89576d7a8c("iproute2: GRE over IPv6 tunnel support.")
> Signed-off-by: Craig Gallek <kraig@google.com>

Thanks. Applied.

^ permalink raw reply

* Re: [PATCH iproute2] iplink: Expose IFLA_*_FWMARK attributes for supported link types
From: Stephen Hemminger @ 2017-04-23 16:16 UTC (permalink / raw)
  To: Craig Gallek; +Cc: netdev
In-Reply-To: <20170421181453.166316-1-kraigatgoog@gmail.com>

On Fri, 21 Apr 2017 14:14:53 -0400
Craig Gallek <kraigatgoog@gmail.com> wrote:

> From: Craig Gallek <kraig@google.com>
> 
> This attribute allows the administrator to adjust the packet marking
> attribute of tunnels that support policy based routing.
> 
> Signed-off-by: Craig Gallek <kraig@google.com>

Applied to net-next. Since the link attributes are not in 4.11

^ permalink raw reply

* Re: [Intel-wired-lan] NFS over NAT causes e1000e transmit hangs
From: Florian Fainelli @ 2017-04-23 17:08 UTC (permalink / raw)
  To: Neftin, Sasha, Eric Dumazet; +Cc: netdev, intel-wired-lan
In-Reply-To: <81dbcf7d-52f4-4dc0-2355-27198685da7b@intel.com>



On 04/22/2017 11:46 PM, Neftin, Sasha wrote:
> On 4/20/2017 00:15, Florian Fainelli wrote:
>> On 04/19/2017 01:52 AM, Neftin, Sasha wrote:
>>> On 4/18/2017 22:05, Florian Fainelli wrote:
>>>> On 04/18/2017 12:03 PM, Eric Dumazet wrote:
>>>>> On Tue, 2017-04-18 at 11:18 -0700, Florian Fainelli wrote:
>>>>>> Hi,
>>>>>>
>>>>>> I am using NFS over a NAT with two e1000e adapters and with eth1
>>>>>> being
>>>>>> the LAN interface and eth0 the WAN interface. The kernel is Ubuntu's
>>>>>> 16.10 kernel: 4.8.0-46-generic. The device doing NAT over NFS is just
>>>>>> mounting a remote folder and doing normal execution/file accesses.
>>>>>> It's
>>>>>> enough to untar a file from this device onto a NFS share to expose
>>>>>> the
>>>>>> problem.
>>>>>>
>>>>>> The transmit hangs look like the ones below, doing a rmmod/insmod
>>>>>> does
>>>>>> not help eliminated the problem, nor does a power cycle. Stopping the
>>>>>> NFS over NAT definitively does let the adapter recover.
>>>>> Is this NFS over TCP or UDP ?
>>>> This is NFS over TCP mounted with the following:
>>>>
>>>> type nfs
>>>> (rw,relatime,vers=3,rsize=1048576,wsize=1048576,namlen=255,hard,proto=tcp,port=2049,timeo=70,retrans=3,sec=sys,local_lock=none,addr=X.X.X.X)
>>>>
>>>>
>>>>
>>>> Thanks Eric!
>>> Please, try disable TCP segmentation offload: ethtool -K <adapter>
>>> tso off.
>> I am not able to reproduce the hangs with TSO turned off. Is there a
>> specific patch you would want me to try?
> 
> Please, work with TSO turned off so. There is no patch for this specific
> problem.

OK, are not we interested in somehow being able to identify such
problematic packets coming from the networking stack and force not using
TSO for those? Would an acceptable solution be to force the disabling of
TSO for this specific NIC model (provided it is some kind of HW bug)?

NB: I understand this is very old hardware for you at Intel, but
conversely, it is very widespread, and chances of people running into
similar issues are pretty high, so fixing it once would de-facto lower
the amount of support you'd have to provide in the future.

Thanks
-- 
Florian

^ permalink raw reply

* [PATCH net-next 1/1] net sched actions: Complete the JUMPX opcode
From: Jamal Hadi Salim @ 2017-04-23 17:17 UTC (permalink / raw)
  To: davem
  Cc: netdev, jiri, xiyou.wangcong, daniel, alexei.starovoitov,
	Jamal Hadi Salim

From: Jamal Hadi Salim <jhs@mojatatu.com>

per discussion at netconf/netdev:
When we have an action that is capable of branching (example a policer),
we can achieve a continuation of the action graph by programming a
"continue" where we find an exact replica of the same filter rule with a lower
priority and the remainder of the action graph. When you have 100s of thousands
of filters which require such a feature it gets very inefficient to do two
lookups.

This patch completes a leftover feature of action codes. Its time has come.

Example below where a user labels packets with a different skbmark on ingress
of a port depending on whether they have/not exceeded the configured rate.
This mark is then used to make further decisions on some egress port.

 #rate control, very low so we can easily see the effect
sudo $TC actions add action police rate 1kbit burst 90k \
conform-exceed pipe/jump 2 index 10
 # skbedit index 11 will be used if the user conforms
sudo $TC actions add action skbedit mark 11 ok index 11
 # skbedit index 12 will be used if the user does not conform
sudo $TC actions add action skbedit mark 12 ok index 12

 #lets bind the user ..
sudo $TC filter add dev $ETH parent ffff: protocol ip prio 8 u32 \
match ip dst 127.0.0.8/32 flowid 1:10 \
action police index 10 \
action skbedit index 11 \
action skbedit index 12

 #run a ping -f and see what happens..
 #
jhs@foobar:~$ sudo $TC -s filter ls dev $ETH parent ffff: protocol ip
filter pref 8 u32
filter pref 8 u32 fh 800: ht divisor 1
filter pref 8 u32 fh 800::800 order 2048 key ht 800 bkt 0 flowid 1:10  (rule hit 2800 success 1005)
  match 7f000008/ffffffff at 16 (success 1005 )
	action order 1:  police 0xa rate 1Kbit burst 23440b mtu 2Kb action pipe/jump 2 overhead 0b
	ref 2 bind 1 installed 207 sec used 122 sec
	Action statistics:
	Sent 84420 bytes 1005 pkt (dropped 0, overlimits 721 requeues 0)
	backlog 0b 0p requeues 0

	action order 2:  skbedit mark 11 pass
	 index 11 ref 2 bind 1 installed 204 sec used 122 sec
 	Action statistics:
	Sent 60564 bytes 721 pkt (dropped 0, overlimits 0 requeues 0)
	backlog 0b 0p requeues 0

	action order 3:  skbedit mark 12 pass
	 index 12 ref 2 bind 1 installed 201 sec used 122 sec
 	Action statistics:
	Sent 23856 bytes 284 pkt (dropped 0, overlimits 0 requeues 0)
	backlog 0b 0p requeues 0

Not bad, about 28% non-conforming packets..

Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 net/sched/act_api.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 257360f..7f2cd70 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -428,24 +428,49 @@ static struct tc_action_ops *tc_lookup_action(struct nlattr *kind)
 	return res;
 }
 
+/*TCA_ACT_MAX_PRIO is 32, there count upto 32 */
+#define TCA_ACT_MAX_PRIO_MASK 0x1FF
 int tcf_action_exec(struct sk_buff *skb, struct tc_action **actions,
 		    int nr_actions, struct tcf_result *res)
 {
 	int ret = -1, i;
+	u32 jmp_prgcnt = 0;
+	u32 jmp_ttl = TCA_ACT_MAX_PRIO; /*matches actions per filter */
 
 	if (skb_skip_tc_classify(skb))
 		return TC_ACT_OK;
 
+restart_act_graph:
 	for (i = 0; i < nr_actions; i++) {
 		const struct tc_action *a = actions[i];
 
+		if (jmp_prgcnt > 0) {
+			jmp_prgcnt -= 1;
+			continue;
+		}
 repeat:
 		ret = a->ops->act(skb, a, res);
 		if (ret == TC_ACT_REPEAT)
 			goto repeat;	/* we need a ttl - JHS */
+
+		if (ret & TC_ACT_JUMP) {
+			jmp_prgcnt = ret & TCA_ACT_MAX_PRIO_MASK;
+			if (!jmp_prgcnt || (jmp_prgcnt > nr_actions)) {
+				/* faulty opcode, stop pipeline */
+				return TC_ACT_OK;
+			} else {
+				jmp_ttl -= 1;
+				if (jmp_ttl > 0)
+					goto restart_act_graph;
+				else /* faulty graph, stop pipeline */
+					return TC_ACT_OK;
+			}
+		}
+
 		if (ret != TC_ACT_PIPE)
 			break;
 	}
+
 	return ret;
 }
 EXPORT_SYMBOL(tcf_action_exec);
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH] net: can: usb: gs_usb: Fix buffer on stack
From: Maksim Salau @ 2017-04-23 17:18 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: Wolfgang Grandegger, Marc Kleine-Budde, Maximilian Schneider,
	Hubert Denkmair, Wolfram Sang, Ethan Zonca, linux-can,
	netdev@vger.kernel.org
In-Reply-To: <CAOMZO5DLdR00FY01r2z=RvSrn4vNEZ+Z3OS0KWB95DGt=3fh2w@mail.gmail.com>

> > +       struct gs_identify_mode *imode = NULL;  
> 
> No need to assign imode to NULL here.

Thanks, Fabio.

I'll fix this in v2.

Maksim

^ permalink raw reply

* Re: [PATCH iproute2 1/1] actions: Add support for user cookies
From: Jamal Hadi Salim @ 2017-04-23 17:18 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, Lucas Bates
In-Reply-To: <20170423091109.79d47f6b@xeon-e3>

On 17-04-23 12:11 PM, Stephen Hemminger wrote:
> On Sat, 22 Apr 2017 08:36:23 -0400

>
> Applied. Please update man page as well.
>
>

Will do.

cheers,
jamal

^ permalink raw reply

* Re: [Intel-wired-lan] NFS over NAT causes e1000e transmit hangs
From: Eric Dumazet @ 2017-04-23 17:24 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: Neftin, Sasha, netdev, intel-wired-lan
In-Reply-To: <09a5327d-0cf9-c4ed-5383-2f02c658437e@gmail.com>

On Sun, 2017-04-23 at 10:08 -0700, Florian Fainelli wrote:
> 
> On 04/22/2017 11:46 PM, Neftin, Sasha wrote:
> > On 4/20/2017 00:15, Florian Fainelli wrote:
> >> On 04/19/2017 01:52 AM, Neftin, Sasha wrote:
> >>> On 4/18/2017 22:05, Florian Fainelli wrote:
> >>>> On 04/18/2017 12:03 PM, Eric Dumazet wrote:
> >>>>> On Tue, 2017-04-18 at 11:18 -0700, Florian Fainelli wrote:
> >>>>>> Hi,
> >>>>>>
> >>>>>> I am using NFS over a NAT with two e1000e adapters and with eth1
> >>>>>> being
> >>>>>> the LAN interface and eth0 the WAN interface. The kernel is Ubuntu's
> >>>>>> 16.10 kernel: 4.8.0-46-generic. The device doing NAT over NFS is just
> >>>>>> mounting a remote folder and doing normal execution/file accesses.
> >>>>>> It's
> >>>>>> enough to untar a file from this device onto a NFS share to expose
> >>>>>> the
> >>>>>> problem.
> >>>>>>
> >>>>>> The transmit hangs look like the ones below, doing a rmmod/insmod
> >>>>>> does
> >>>>>> not help eliminated the problem, nor does a power cycle. Stopping the
> >>>>>> NFS over NAT definitively does let the adapter recover.
> >>>>> Is this NFS over TCP or UDP ?
> >>>> This is NFS over TCP mounted with the following:
> >>>>
> >>>> type nfs
> >>>> (rw,relatime,vers=3,rsize=1048576,wsize=1048576,namlen=255,hard,proto=tcp,port=2049,timeo=70,retrans=3,sec=sys,local_lock=none,addr=X.X.X.X)
> >>>>
> >>>>
> >>>>
> >>>> Thanks Eric!
> >>> Please, try disable TCP segmentation offload: ethtool -K <adapter>
> >>> tso off.
> >> I am not able to reproduce the hangs with TSO turned off. Is there a
> >> specific patch you would want me to try?
> > 
> > Please, work with TSO turned off so. There is no patch for this specific
> > problem.
> 
> OK, are not we interested in somehow being able to identify such
> problematic packets coming from the networking stack and force not using
> TSO for those? Would an acceptable solution be to force the disabling of
> TSO for this specific NIC model (provided it is some kind of HW bug)?
> 
> NB: I understand this is very old hardware for you at Intel, but
> conversely, it is very widespread, and chances of people running into
> similar issues are pretty high, so fixing it once would de-facto lower
> the amount of support you'd have to provide in the future.

Indeed it is very odd to disable TSO, especially if the problem only
shows up with NAT.

We probably have a nasty bug somewhere, or we might be able to have a
work around some hardware 'feature'.

^ permalink raw reply

* [PATCH v2] net: can: usb: gs_usb: Fix buffer on stack
From: Maksim Salau @ 2017-04-23 17:31 UTC (permalink / raw)
  To: Wolfgang Grandegger, Marc Kleine-Budde, Maximilian Schneider,
	Hubert Denkmair, Wolfram Sang, Ethan Zonca, linux-can, netdev,
	Fabio Estevam
  Cc: Maksim Salau
In-Reply-To: <CAOMZO5DLdR00FY01r2z=RvSrn4vNEZ+Z3OS0KWB95DGt=3fh2w@mail.gmail.com>

Allocate buffers on HEAP instead of STACK for local structures
that are to be sent using usb_control_msg().

Signed-off-by: Maksim Salau <maksim.salau@gmail.com>
---
 Changes in v2:
   dropped redundant assignment.

 drivers/net/can/usb/gs_usb.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
index 300349f..eecee7f 100644
--- a/drivers/net/can/usb/gs_usb.c
+++ b/drivers/net/can/usb/gs_usb.c
@@ -739,13 +739,18 @@ static const struct net_device_ops gs_usb_netdev_ops = {
 static int gs_usb_set_identify(struct net_device *netdev, bool do_identify)
 {
 	struct gs_can *dev = netdev_priv(netdev);
-	struct gs_identify_mode imode;
+	struct gs_identify_mode *imode;
 	int rc;
 
+	imode = kmalloc(sizeof(*imode), GFP_KERNEL);
+
+	if (!imode)
+		return -ENOMEM;
+
 	if (do_identify)
-		imode.mode = GS_CAN_IDENTIFY_ON;
+		imode->mode = GS_CAN_IDENTIFY_ON;
 	else
-		imode.mode = GS_CAN_IDENTIFY_OFF;
+		imode->mode = GS_CAN_IDENTIFY_OFF;
 
 	rc = usb_control_msg(interface_to_usbdev(dev->iface),
 			     usb_sndctrlpipe(interface_to_usbdev(dev->iface),
@@ -755,10 +760,12 @@ static int gs_usb_set_identify(struct net_device *netdev, bool do_identify)
 			     USB_RECIP_INTERFACE,
 			     dev->channel,
 			     0,
-			     &imode,
-			     sizeof(imode),
+			     imode,
+			     sizeof(*imode),
 			     100);
 
+	kfree(imode);
+
 	return (rc > 0) ? 0 : rc;
 }
 
-- 
2.9.3


^ permalink raw reply related

* Re: [PATCH net-next] bpf, doc: update list of architectures that do eBPF JIT
From: Daniel Borkmann @ 2017-04-23 17:39 UTC (permalink / raw)
  To: Alexei Starovoitov, David S . Miller; +Cc: netdev
In-Reply-To: <20170423160100.1155435-1-ast@fb.com>

On 04/23/2017 06:01 PM, Alexei Starovoitov wrote:
> update the list and remove 'in the future' statement,
> since all still alive 64-bit architectures now do eBPF JIT.
>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>

Acked-by: Daniel Borkmann <daniel@iogearbox.net>

> mips64 is the only 'still alive' 64-bit arch without eBPF JIT :)

(... and work in progress by cavium folks. ;))

^ permalink raw reply

* Re: [PATCH iproute2 net 3/8] tc/pedit: Introduce 'add' operation
From: Jamal Hadi Salim @ 2017-04-23 17:44 UTC (permalink / raw)
  To: Amir Vadai, Stephen Hemminger; +Cc: netdev, Or Gerlitz
In-Reply-To: <20170423125356.1298-4-amir@vadai.me>


Thanks for the excellent work.

On 17-04-23 08:53 AM, Amir Vadai wrote:
> This command could be useful to increase/decrease fields value.
>

Does this contradict the "retain" feature? Example rule to
retain the second nibble but set the first to 0xA
(essentially it X-ORs):
tc filter add dev lo parent ffff: protocol ip prio 10 \
u32 match ip dst 127.0.0.1/32 flowid 1:1 \
action pedit munge offset 0 u8 set 0x0A retain 0xF0

Mostly curious about hardware handling.

cheers,
jamal

^ permalink raw reply

* [PATCH 1/1] drivers:net:ethernet:adi:bfin_mac: Use FIELD_SIZEOF defined kernel macro
From: Karim Eshapa @ 2017-04-23 18:02 UTC (permalink / raw)
  To: davem
  Cc: tremyfr, a, fw, clabbe.montjoie, edumazet, jarod,
	adi-buildroot-devel, netdev, linux-kernel, Karim Eshapa

Use FIELD_SIZEOF defined kernel macro kernel.h

Signed-off-by: Karim Eshapa <karim.eshapa@gmail.com>
---
 drivers/net/ethernet/adi/bfin_mac.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/adi/bfin_mac.c b/drivers/net/ethernet/adi/bfin_mac.c
index a9ac58c..60346e0 100644
--- a/drivers/net/ethernet/adi/bfin_mac.c
+++ b/drivers/net/ethernet/adi/bfin_mac.c
@@ -452,10 +452,14 @@ static irqreturn_t bfin_mac_wake_interrupt(int irq, void *dev_id)
 static void bfin_mac_ethtool_getdrvinfo(struct net_device *dev,
 					struct ethtool_drvinfo *info)
 {
-	strlcpy(info->driver, KBUILD_MODNAME, sizeof(info->driver));
-	strlcpy(info->version, DRV_VERSION, sizeof(info->version));
-	strlcpy(info->fw_version, "N/A", sizeof(info->fw_version));
-	strlcpy(info->bus_info, dev_name(&dev->dev), sizeof(info->bus_info));
+	strlcpy(info->driver, KBUILD_MODNAME, FIELD_SIZEOF(
+		struct ethtool_drvinfo, driver));
+	strlcpy(info->version, DRV_VERSION, FIELD_SIZEOF(
+		struct ethtool_drvinfo, version));
+	strlcpy(info->fw_version, "N/A", FIELD_SIZEOF(
+		struct ethtool_drvinfo, fw_version));
+	strlcpy(info->bus_info, dev_name(&dev->dev), FIELD_SIZEOF(
+		struct ethtool_drvinfo, bus_info));
 }
 
 static void bfin_mac_ethtool_getwol(struct net_device *dev,
@@ -785,7 +789,7 @@ static int bfin_mac_hwtstamp_get(struct net_device *netdev,
 	struct bfin_mac_local *lp = netdev_priv(netdev);
 
 	return copy_to_user(ifr->ifr_data, &lp->stamp_cfg,
-			    sizeof(lp->stamp_cfg)) ?
+			    FILD_SIZEOF(struct bfin_mac_local, stamp_cfg)) ?
 		-EFAULT : 0;
 }
 
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH net] ravb: Double free on error in ravb_start_xmit()
From: Sergei Shtylyov @ 2017-04-23 18:16 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: David S. Miller, Kazuya Mizuguchi, Simon Horman, Yoshihiro Kaneko,
	Masaru Nagai, Geert Uytterhoeven, Niklas Söderlund,
	Philippe Reynes, netdev, linux-renesas-soc, kernel-janitors
In-Reply-To: <20170422104656.jjnooh3or4x3lbw3@mwanda>

Hello!

On 04/22/2017 01:46 PM, Dan Carpenter wrote:

> If skb_put_padto() fails then it frees the skb.  I shifted that code
> up a bit to make my error handling a little simpler.
>
> Fixes: a0d2f20650e8 ("Renesas Ethernet AVB PTP clock driver")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Acked-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

MBR, Sergei

^ permalink raw reply

* Re: [PATCH 1/1] cfg80211: add return value validation
From: Johannes Berg @ 2017-04-23 18:41 UTC (permalink / raw)
  To: Pan Bian, Jussi Kivilinna, Kalle Valo,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Pan Bian
In-Reply-To: <1492954023-8596-1-git-send-email-bianpan201603-9Onoh4P/yGk@public.gmane.org>

This is not a cfg80211 patch, please resend with the correct subject.

Thanks,
johannes

^ permalink raw reply

* Re: [PATCH] brcm80211: brcmfmac: Ensure that incoming skb's are writable
From: Arend Van Spriel @ 2017-04-23 19:34 UTC (permalink / raw)
  To: James Hughes
  Cc: netdev, Franky Lin, Hante Meuleman, Kalle Valo, linux-wireless
In-Reply-To: <CAE_XsMKSO=P-h5AyPJy=59zDq=DmLFqBAX8Bqsofey=aAFD5dQ@mail.gmail.com>

On 21-4-2017 11:22, James Hughes wrote:
> On 20 April 2017 at 20:48, Arend van Spriel
> <arend.vanspriel@broadcom.com> wrote:
>> + linux-wireless
>>
>> On 4/20/2017 1:16 PM, James Hughes wrote:
>>>
>>> The driver was adding header information to incoming skb
>>> without ensuring the head was uncloned and hence writable.
>>>
>>> skb_cow_head has been used to ensure they are writable, however,
>>> this required some changes to error handling to ensure that
>>> if skb_cow_head failed it was not ignored.
>>>
>>> This really needs to be reviewed by someone who is more familiar
>>> with this code base to ensure any deallocation of skb's is
>>> still correct.
>>>
>>> Signed-off-by: James Hughes <james.hughes@raspberrypi.org>
>>> ---
>>>   .../wireless/broadcom/brcm80211/brcmfmac/bcdc.c    | 15 ++++++++--
>>>   .../wireless/broadcom/brcm80211/brcmfmac/core.c    | 23 +++++-----------
>>>   .../broadcom/brcm80211/brcmfmac/fwsignal.c         | 32
>>> +++++++++++++++++-----
>>>   .../wireless/broadcom/brcm80211/brcmfmac/sdio.c    |  7 ++++-
>>>   4 files changed, 51 insertions(+), 26 deletions(-)
>>>
>>
>> [...]
>>
>>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
>>> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
>>> index 5eaac13..08272e8 100644
>>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
>>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
>>> @@ -198,7 +198,7 @@ static netdev_tx_t brcmf_netdev_start_xmit(struct
>>> sk_buff *skb,
>>>         int ret;
>>>         struct brcmf_if *ifp = netdev_priv(ndev);
>>>         struct brcmf_pub *drvr = ifp->drvr;
>>> -       struct ethhdr *eh = (struct ethhdr *)(skb->data);
>>> +       struct ethhdr *eh;
>>>         brcmf_dbg(DATA, "Enter, bsscfgidx=%d\n", ifp->bsscfgidx);
>>>   @@ -212,23 +212,14 @@ static netdev_tx_t brcmf_netdev_start_xmit(struct
>>> sk_buff *skb,
>>>         }
>>>         /* Make sure there's enough room for any header */
>>> -       if (skb_headroom(skb) < drvr->hdrlen) {
>>> -               struct sk_buff *skb2;
>>> -
>>> -               brcmf_dbg(INFO, "%s: insufficient headroom\n",
>>> -                         brcmf_ifname(ifp));
>>> -               drvr->bus_if->tx_realloc++;
>>> -               skb2 = skb_realloc_headroom(skb, drvr->hdrlen);
>>> -               dev_kfree_skb(skb);
>>> -               skb = skb2;
>>> -               if (skb == NULL) {
>>> -                       brcmf_err("%s: skb_realloc_headroom failed\n",
>>> -                                 brcmf_ifname(ifp));
>>> -                       ret = -ENOMEM;
>>> -                       goto done;
>>> -               }
>>
>>
>> What you are throwing away here is code that assures there is sufficient
>> headroom for protocol and bus layer in the tx path, because that is
>> determined by drvr->hdrlen. This is where the skb is handed to the driver so
>> if you could leave the functionality above *and* assure it is writeable that
>> would be the best solution as there is no need for all the other changes
>> down the tx path.
> 
> The skb_cow_head function takes the required headroom as a parameter
> and will ensure that there is enough space, so I don't think this code
> segment is
> required or have I misunderstood what you mean here?

I looked more into what skb_cow_head() and skb_realloc_headroom()
actually do and it seems you are right.

> Is it safe to rely on the _cow_ being done here and not further down
> in the stack?
> Or at least checked further down in the stack. Previous comments from
> another patch
> requested that the _cow_ be done close to the actual addition of the
> header. I presume
> it is unlikely/impossible that the functions that add header
> information  down the stack
> will be called without the above being done first?

It is safe. During probe sequence each layer in the driver stack
increments drvr->hdrlen with headroom it needs. So drvr->hdrlen will
indicate the total headroom needed when .start_xmit() is called. And
yes, the .start_xmit() is the single point of entry in the transmit
path. So the other locations where skb_push() is done are safe.

>>
>>> +       ret = skb_cow_head(skb, drvr->hdrlen);
>>> +       if (ret) {
>>
>>
>> So move the realloc code above here instead of simply freeing the skb.
>>
>>> +               dev_kfree_skb_any(skb);
>>> +               goto done;
>>>         }
>>>   +     eh = (struct ethhdr *)(skb->data);
>>
>>
>> Now this is actually a separate fix so I would like a separate patch for it.
>>
> 
> No problem, but see final paragraph below.

Let me see...

>> I have a RPi3 sitting on my desk so how can I replicate the issue. It was
>> something about broadcast/multicast traffic when using AP mode and a bridge,
>> right?
>>
>> Regards,
>> Arend
> 
> See this issue for details on replication.
> https://github.com/raspberrypi/firmware/issues/673
> 
> The bridge I use is setup using a similar procedure to that described
> here.  https://www.raspberrypi.org/documentation/configuration/wireless/access-point.md
> 
> I'm happy to pass over this work to you guys if you think it is
> appropriate, you are the
> experts on the codebase.

Sure. However, I must say you did an excellent job digging into the
issue and root causing it. We always were under the impression that we
could treat the skb as our own when handed to us in .start_xmit() callback.

Regards,
Arend

^ permalink raw reply


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