* Re: [PATCH net-next v2] vxlan: change vxlan_[config_]validate() to use netlink_ext_ack for error reporting
From: Jiri Benc @ 2017-08-11 16:39 UTC (permalink / raw)
To: Roopa Prabhu
Cc: Girish Moodalbail, pravin shelar, davem@davemloft.net,
netdev@vger.kernel.org, Matthias Schiffer
In-Reply-To: <CAJieiUjYggf3zNzwBMFq7zUm+RrZokZ7oNpuz-Hinec=0OSGzA@mail.gmail.com>
On Fri, 11 Aug 2017 09:19:34 -0700, Roopa Prabhu wrote:
> > if (tb[IFLA_ADDRESS]) {
> > if (nla_len(tb[IFLA_ADDRESS]) != ETH_ALEN) {
> > - pr_debug("invalid link address (not ethernet)\n");
> > + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_ADDRESS],
> > + "Provided link layer address is not Ethernet");
> > return -EINVAL;
> > }
>
> keep it simple and closer to the original msg: "invalid link layer address"
I prefer more explanatory wording. Girish's is better.
> >
> > if (!is_valid_ether_addr(nla_data(tb[IFLA_ADDRESS]))) {
> > - pr_debug("invalid all zero ethernet address\n");
> > + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_ADDRESS],
> > + "Provided Ethernet address is not unicast");
> > return -EADDRNOTAVAIL;
>
> keep it simple and closer to the original msg: "invalid all zero
> ethernet address"
This would be incorrect message. The is_valid_ether_addr function does
not check only for all zeroes but also for !multicast. Girish's wording
better expresses what's going on.
> > + if (!data) {
> > + NL_SET_ERR_MSG(extack,
> > + "Not enough attributes provided to perform the operation");
> > return -EINVAL;
> > + }
>
> "not enough attributes"
You're missing part of the sentence.
> > if (data[IFLA_VXLAN_ID]) {
> > u32 id = nla_get_u32(data[IFLA_VXLAN_ID]);
> >
> > - if (id >= VXLAN_N_VID)
> > + if (id >= VXLAN_N_VID) {
> > + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_ID],
> > + "VXLAN ID must be lower than 16777216");
> > return -ERANGE;
>
> "invalid vxlan-id"
This is exactly what I objected against in Girish's v1. It would be
useless to have extended error reporting but report things that don't
help users. I like the current Girish's wording, it's clear and helpful.
> > @@ -2761,8 +2772,8 @@ static int vxlan_validate(struct nlattr *tb[], struct nlattr *data[],
> > = nla_data(data[IFLA_VXLAN_PORT_RANGE]);
> >
> > if (ntohs(p->high) < ntohs(p->low)) {
> > - pr_debug("port range %u .. %u not valid\n",
> > - ntohs(p->low), ntohs(p->high));
> > + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_PORT_RANGE],
> > + "Provided source port range bounds is invalid");
> > return -EINVAL;
> > }
>
> "invalid source port range"
Could be. But please honor proper capitalization.
> > @@ -2933,6 +2945,8 @@ static int vxlan_config_validate(struct net *src_net, struct vxlan_config *conf,
> > */
> > if ((conf->flags & ~VXLAN_F_ALLOWED_GPE) ||
> > !(conf->flags & VXLAN_F_COLLECT_METADATA)) {
> > + NL_SET_ERR_MSG(extack,
> > + "VXLAN GPE does not support this combination of attributes");
> > return -EINVAL;
> > }
>
> "collect metadata not supported with vxlan gpe"
That's completely wrong message. Not saying that the capitalization is
wrong, too. Girish's wording precisely explains what went wrong.
> > - if (vxlan_addr_multicast(&conf->saddr))
> > + if (vxlan_addr_multicast(&conf->saddr)) {
> > + NL_SET_ERR_MSG(extack, "Local address cannot be multicast");
> > return -EINVAL;
>
> "invalid local address. multicast not supported"
Roopa, what happened to your shift key? And how is this better to what
Girish proposed?
> > + NL_SET_ERR_MSG(extack,
> > + "IPv6 support not enabled in the kernel");
>
> "invalid address family. ipv6 not enabled"
Ditto.
> > lowerdev = __dev_get_by_index(src_net, conf->remote_ifindex);
> > - if (!lowerdev)
> > + if (!lowerdev) {
> > + NL_SET_ERR_MSG(extack,
> > + "Specified interface for tunnel endpoint communications not found");
> > return -ENODEV;
>
> "invalid vxlan remote link interface, device not found"
Finally one that looks better :-) Modulo the missing capitalization,
though.
> > - if (vxlan_addr_multicast(&conf->remote_ip))
> > + if (vxlan_addr_multicast(&conf->remote_ip)) {
> > + NL_SET_ERR_MSG(extack,
> > + "Interface need to be specified for multicast destination");
>
> "vxlan remote link interface required for multicast remote destination"
I like this one better, too.
> > #if IS_ENABLED(CONFIG_IPV6)
> > - if (conf->flags & VXLAN_F_IPV6_LINKLOCAL)
> > + if (conf->flags & VXLAN_F_IPV6_LINKLOCAL) {
> > + NL_SET_ERR_MSG(extack,
> > + "Interface need to be specified for link-local local/remote addresses");
> > return -EINVAL;
>
> "vxlan link interface required for link-local local/remote addresses"
Okay but to be consistent (and more clear), it should be "remote link
interface".
> > @@ -3038,6 +3082,7 @@ static int vxlan_config_validate(struct net *src_net, struct vxlan_config *conf,
> > tmp->cfg.remote_ifindex != conf->remote_ifindex)
> > continue;
> >
> > + NL_SET_ERR_MSG(extack, "Specified VNI is duplicate");
>
> "duplicate vni. vxlan device with vni exists."
What about "A VXLAN device with the specified VNI already exists."?
Jiri
^ permalink raw reply
* Re: [PATCH net-next v2] vxlan: change vxlan_[config_]validate() to use netlink_ext_ack for error reporting
From: Matthias Schiffer @ 2017-08-11 16:24 UTC (permalink / raw)
To: Roopa Prabhu
Cc: Girish Moodalbail, pravin shelar, davem@davemloft.net,
netdev@vger.kernel.org, Jiri Benc
In-Reply-To: <CAJieiUjYggf3zNzwBMFq7zUm+RrZokZ7oNpuz-Hinec=0OSGzA@mail.gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 713 bytes --]
On 08/11/2017 06:19 PM, Roopa Prabhu wrote:
> On Thu, Aug 10, 2017 at 2:16 PM, Girish Moodalbail
> <girish.moodalbail@oracle.com> wrote:
>>
>> if (!is_valid_ether_addr(nla_data(tb[IFLA_ADDRESS]))) {
>> - pr_debug("invalid all zero ethernet address\n");
>> + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_ADDRESS],
>> + "Provided Ethernet address is not unicast");
>> return -EADDRNOTAVAIL;
>
>
> keep it simple and closer to the original msg: "invalid all zero
> ethernet address"
It is inaccurate though, is_valid_ether_addr will also reject multicast
addresses.
Matthias
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH net 2/2] bpf: fix two missing target_size settings in bpf_convert_ctx_access
From: Daniel Borkmann @ 2017-08-11 16:31 UTC (permalink / raw)
To: davem; +Cc: ast, netdev, Daniel Borkmann
In-Reply-To: <cover.1502468639.git.daniel@iogearbox.net>
When CONFIG_NET_SCHED or CONFIG_NET_RX_BUSY_POLL is /not/ set and
we try a narrow __sk_buff load of tc_index or napi_id, respectively,
then verifier rightfully complains that it's misconfigured, because
we need to set target_size in each of the two cases. The rewrite
for the ctx access is just a dummy op, but needs to pass, so fix
this up.
Fixes: f96da09473b5 ("bpf: simplify narrower ctx access")
Reported-by: Shubham Bansal <illusionist.neo@gmail.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
net/core/filter.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/core/filter.c b/net/core/filter.c
index f44fc22..6280a60 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -3505,6 +3505,7 @@ static u32 bpf_convert_ctx_access(enum bpf_access_type type,
bpf_target_off(struct sk_buff, tc_index, 2,
target_size));
#else
+ *target_size = 2;
if (type == BPF_WRITE)
*insn++ = BPF_MOV64_REG(si->dst_reg, si->dst_reg);
else
@@ -3520,6 +3521,7 @@ static u32 bpf_convert_ctx_access(enum bpf_access_type type,
*insn++ = BPF_JMP_IMM(BPF_JGE, si->dst_reg, MIN_NAPI_ID, 1);
*insn++ = BPF_MOV64_IMM(si->dst_reg, 0);
#else
+ *target_size = 4;
*insn++ = BPF_MOV64_IMM(si->dst_reg, 0);
#endif
break;
--
1.9.3
^ permalink raw reply related
* [PATCH net 1/2] net: fix compilation when busy poll is not enabled
From: Daniel Borkmann @ 2017-08-11 16:31 UTC (permalink / raw)
To: davem; +Cc: ast, netdev, Daniel Borkmann
In-Reply-To: <cover.1502468639.git.daniel@iogearbox.net>
MIN_NAPI_ID is used in various places outside of
CONFIG_NET_RX_BUSY_POLL wrapping, so when it's not set
we run into build errors such as:
net/core/dev.c: In function 'dev_get_by_napi_id':
net/core/dev.c:886:16: error: ‘MIN_NAPI_ID’ undeclared (first use in this function)
if (napi_id < MIN_NAPI_ID)
^~~~~~~~~~~
Thus, have MIN_NAPI_ID always defined to fix these errors.
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
include/net/busy_poll.h | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/include/net/busy_poll.h b/include/net/busy_poll.h
index 8ffd434..71c72a9 100644
--- a/include/net/busy_poll.h
+++ b/include/net/busy_poll.h
@@ -29,18 +29,18 @@
#include <linux/sched/signal.h>
#include <net/ip.h>
-#ifdef CONFIG_NET_RX_BUSY_POLL
-
-struct napi_struct;
-extern unsigned int sysctl_net_busy_read __read_mostly;
-extern unsigned int sysctl_net_busy_poll __read_mostly;
-
/* 0 - Reserved to indicate value not set
* 1..NR_CPUS - Reserved for sender_cpu
* NR_CPUS+1..~0 - Region available for NAPI IDs
*/
#define MIN_NAPI_ID ((unsigned int)(NR_CPUS + 1))
+#ifdef CONFIG_NET_RX_BUSY_POLL
+
+struct napi_struct;
+extern unsigned int sysctl_net_busy_read __read_mostly;
+extern unsigned int sysctl_net_busy_poll __read_mostly;
+
static inline bool net_busy_loop_on(void)
{
return sysctl_net_busy_poll;
--
1.9.3
^ permalink raw reply related
* [PATCH net 0/2] Minor fix in bpf_convert_ctx_access
From: Daniel Borkmann @ 2017-08-11 16:31 UTC (permalink / raw)
To: davem; +Cc: ast, netdev, Daniel Borkmann
First one was found while trying to compile the kernel
with !CONFIG_NET_RX_BUSY_POLL.
Thanks!
Daniel Borkmann (2):
net: fix compilation when busy poll is not enabled
bpf: fix two missing target_size settings in bpf_convert_ctx_access
include/net/busy_poll.h | 12 ++++++------
net/core/filter.c | 2 ++
2 files changed, 8 insertions(+), 6 deletions(-)
--
1.9.3
^ permalink raw reply
* Re: [PATCH net-next v2] vxlan: change vxlan_[config_]validate() to use netlink_ext_ack for error reporting
From: Roopa Prabhu @ 2017-08-11 16:19 UTC (permalink / raw)
To: Girish Moodalbail
Cc: pravin shelar, davem@davemloft.net, netdev@vger.kernel.org,
Matthias Schiffer, Jiri Benc
In-Reply-To: <1502399795-25660-1-git-send-email-girish.moodalbail@oracle.com>
On Thu, Aug 10, 2017 at 2:16 PM, Girish Moodalbail
<girish.moodalbail@oracle.com> wrote:
> The kernel log is not where users expect error messages for netlink
> requests; as we have extended acks now, we can replace pr_debug() with
> NL_SET_ERR_MSG_ATTR().
>
> Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net>
> Signed-off-by: Girish Moodalbail <girish.moodalbail@oracle.com>
>
> ---
> v1 -> v2:
> - addressed, error messages rewording, comments from Jiri Benc
> - started off with what Matthias had, and I covered error reporting
> for all of the unsuccessful returns
I have a similar patch in my tree, so, i am tempted to suggest
alternate wordings for some of the msgs below :)
> ---
> drivers/net/vxlan.c | 98 +++++++++++++++++++++++++++++++++++++++--------------
> 1 file changed, 72 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> index 35e84a9e..ec302cd7 100644
> --- a/drivers/net/vxlan.c
> +++ b/drivers/net/vxlan.c
> @@ -2729,12 +2729,14 @@ static int vxlan_validate(struct nlattr *tb[], struct nlattr *data[],
> {
> if (tb[IFLA_ADDRESS]) {
> if (nla_len(tb[IFLA_ADDRESS]) != ETH_ALEN) {
> - pr_debug("invalid link address (not ethernet)\n");
> + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_ADDRESS],
> + "Provided link layer address is not Ethernet");
> return -EINVAL;
> }
keep it simple and closer to the original msg: "invalid link layer address"
>
> if (!is_valid_ether_addr(nla_data(tb[IFLA_ADDRESS]))) {
> - pr_debug("invalid all zero ethernet address\n");
> + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_ADDRESS],
> + "Provided Ethernet address is not unicast");
> return -EADDRNOTAVAIL;
keep it simple and closer to the original msg: "invalid all zero
ethernet address"
> }
> }
> @@ -2742,18 +2744,27 @@ static int vxlan_validate(struct nlattr *tb[], struct nlattr *data[],
> if (tb[IFLA_MTU]) {
> u32 mtu = nla_get_u32(tb[IFLA_MTU]);
>
> - if (mtu < ETH_MIN_MTU || mtu > ETH_MAX_MTU)
> + if (mtu < ETH_MIN_MTU || mtu > ETH_MAX_MTU) {
> + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_MTU],
> + "MTU must be between 68 and 65535");
> return -EINVAL;
> + }
> }
>
> - if (!data)
> + if (!data) {
> + NL_SET_ERR_MSG(extack,
> + "Not enough attributes provided to perform the operation");
> return -EINVAL;
> + }
"not enough attributes"
>
> if (data[IFLA_VXLAN_ID]) {
> u32 id = nla_get_u32(data[IFLA_VXLAN_ID]);
>
> - if (id >= VXLAN_N_VID)
> + if (id >= VXLAN_N_VID) {
> + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_ID],
> + "VXLAN ID must be lower than 16777216");
> return -ERANGE;
"invalid vxlan-id"
> + }
> }
>
> if (data[IFLA_VXLAN_PORT_RANGE]) {
> @@ -2761,8 +2772,8 @@ static int vxlan_validate(struct nlattr *tb[], struct nlattr *data[],
> = nla_data(data[IFLA_VXLAN_PORT_RANGE]);
>
> if (ntohs(p->high) < ntohs(p->low)) {
> - pr_debug("port range %u .. %u not valid\n",
> - ntohs(p->low), ntohs(p->high));
> + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_PORT_RANGE],
> + "Provided source port range bounds is invalid");
> return -EINVAL;
> }
"invalid source port range"
> }
> @@ -2919,7 +2930,8 @@ static int vxlan_sock_add(struct vxlan_dev *vxlan)
>
> static int vxlan_config_validate(struct net *src_net, struct vxlan_config *conf,
> struct net_device **lower,
> - struct vxlan_dev *old)
> + struct vxlan_dev *old,
> + struct netlink_ext_ack *extack)
> {
> struct vxlan_net *vn = net_generic(src_net, vxlan_net_id);
> struct vxlan_dev *tmp;
> @@ -2933,6 +2945,8 @@ static int vxlan_config_validate(struct net *src_net, struct vxlan_config *conf,
> */
> if ((conf->flags & ~VXLAN_F_ALLOWED_GPE) ||
> !(conf->flags & VXLAN_F_COLLECT_METADATA)) {
> + NL_SET_ERR_MSG(extack,
> + "VXLAN GPE does not support this combination of attributes");
> return -EINVAL;
> }
"collect metadata not supported with vxlan gpe"
> }
> @@ -2947,15 +2961,23 @@ static int vxlan_config_validate(struct net *src_net, struct vxlan_config *conf,
> conf->saddr.sa.sa_family = conf->remote_ip.sa.sa_family;
> }
>
> - if (conf->saddr.sa.sa_family != conf->remote_ip.sa.sa_family)
> + if (conf->saddr.sa.sa_family != conf->remote_ip.sa.sa_family) {
> + NL_SET_ERR_MSG(extack,
> + "Local and remote address must be from the same family");
> return -EINVAL;
> + }
>
> - if (vxlan_addr_multicast(&conf->saddr))
> + if (vxlan_addr_multicast(&conf->saddr)) {
> + NL_SET_ERR_MSG(extack, "Local address cannot be multicast");
> return -EINVAL;
"invalid local address. multicast not supported"
> + }
>
> if (conf->saddr.sa.sa_family == AF_INET6) {
> - if (!IS_ENABLED(CONFIG_IPV6))
> + if (!IS_ENABLED(CONFIG_IPV6)) {
> + NL_SET_ERR_MSG(extack,
> + "IPv6 support not enabled in the kernel");
"invalid address family. ipv6 not enabled"
> return -EPFNOSUPPORT;
> + }
> use_ipv6 = true;
> conf->flags |= VXLAN_F_IPV6;
>
> @@ -2967,46 +2989,68 @@ static int vxlan_config_validate(struct net *src_net, struct vxlan_config *conf,
>
> if (local_type & IPV6_ADDR_LINKLOCAL) {
> if (!(remote_type & IPV6_ADDR_LINKLOCAL) &&
> - (remote_type != IPV6_ADDR_ANY))
> + (remote_type != IPV6_ADDR_ANY)) {
> + NL_SET_ERR_MSG(extack,
> + "Invalid combination of local and remote address scopes");
> return -EINVAL;
> + }
>
> conf->flags |= VXLAN_F_IPV6_LINKLOCAL;
> } else {
> if (remote_type ==
> - (IPV6_ADDR_UNICAST | IPV6_ADDR_LINKLOCAL))
> + (IPV6_ADDR_UNICAST | IPV6_ADDR_LINKLOCAL)) {
> + NL_SET_ERR_MSG(extack,
> + "Invalid combination of local and remote address scopes");
> return -EINVAL;
> + }
>
> conf->flags &= ~VXLAN_F_IPV6_LINKLOCAL;
> }
> }
> }
>
> - if (conf->label && !use_ipv6)
> + if (conf->label && !use_ipv6) {
> + NL_SET_ERR_MSG(extack,
> + "Label attribute only applies for IPv6 VXLAN devices");
> return -EINVAL;
> + }
>
> if (conf->remote_ifindex) {
> struct net_device *lowerdev;
>
> lowerdev = __dev_get_by_index(src_net, conf->remote_ifindex);
> - if (!lowerdev)
> + if (!lowerdev) {
> + NL_SET_ERR_MSG(extack,
> + "Specified interface for tunnel endpoint communications not found");
> return -ENODEV;
"invalid vxlan remote link interface, device not found"
> + }
>
> #if IS_ENABLED(CONFIG_IPV6)
> if (use_ipv6) {
> struct inet6_dev *idev = __in6_dev_get(lowerdev);
> - if (idev && idev->cnf.disable_ipv6)
> + if (idev && idev->cnf.disable_ipv6) {
> + NL_SET_ERR_MSG(extack,
> + "IPv6 support disabled by administrator");
> return -EPERM;
> + }
> }
> #endif
>
> *lower = lowerdev;
> } else {
> - if (vxlan_addr_multicast(&conf->remote_ip))
> + if (vxlan_addr_multicast(&conf->remote_ip)) {
> + NL_SET_ERR_MSG(extack,
> + "Interface need to be specified for multicast destination");
"vxlan remote link interface required for multicast remote destination"
> +
> return -EINVAL;
> + }
>
> #if IS_ENABLED(CONFIG_IPV6)
> - if (conf->flags & VXLAN_F_IPV6_LINKLOCAL)
> + if (conf->flags & VXLAN_F_IPV6_LINKLOCAL) {
> + NL_SET_ERR_MSG(extack,
> + "Interface need to be specified for link-local local/remote addresses");
> return -EINVAL;
"vxlan link interface required for link-local local/remote addresses"
> + }
> #endif
>
> *lower = NULL;
> @@ -3038,6 +3082,7 @@ static int vxlan_config_validate(struct net *src_net, struct vxlan_config *conf,
> tmp->cfg.remote_ifindex != conf->remote_ifindex)
> continue;
>
> + NL_SET_ERR_MSG(extack, "Specified VNI is duplicate");
"duplicate vni. vxlan device with vni exists."
> return -EEXIST;
> }
>
> @@ -3097,14 +3142,14 @@ static void vxlan_config_apply(struct net_device *dev,
> }
>
> static int vxlan_dev_configure(struct net *src_net, struct net_device *dev,
> - struct vxlan_config *conf,
> - bool changelink)
> + struct vxlan_config *conf, bool changelink,
> + struct netlink_ext_ack *extack)
> {
> struct vxlan_dev *vxlan = netdev_priv(dev);
> struct net_device *lowerdev;
> int ret;
>
> - ret = vxlan_config_validate(src_net, conf, &lowerdev, vxlan);
> + ret = vxlan_config_validate(src_net, conf, &lowerdev, vxlan, extack);
> if (ret)
> return ret;
>
> @@ -3114,13 +3159,14 @@ static int vxlan_dev_configure(struct net *src_net, struct net_device *dev,
> }
>
> static int __vxlan_dev_create(struct net *net, struct net_device *dev,
> - struct vxlan_config *conf)
> + struct vxlan_config *conf,
> + struct netlink_ext_ack *extack)
> {
> struct vxlan_net *vn = net_generic(net, vxlan_net_id);
> struct vxlan_dev *vxlan = netdev_priv(dev);
> int err;
>
> - err = vxlan_dev_configure(net, dev, conf, false);
> + err = vxlan_dev_configure(net, dev, conf, false, extack);
> if (err)
> return err;
>
> @@ -3366,7 +3412,7 @@ static int vxlan_newlink(struct net *src_net, struct net_device *dev,
> if (err)
> return err;
>
> - return __vxlan_dev_create(src_net, dev, &conf);
> + return __vxlan_dev_create(src_net, dev, &conf, extack);
> }
>
> static int vxlan_changelink(struct net_device *dev, struct nlattr *tb[],
> @@ -3386,7 +3432,7 @@ static int vxlan_changelink(struct net_device *dev, struct nlattr *tb[],
>
> memcpy(&old_dst, dst, sizeof(struct vxlan_rdst));
>
> - err = vxlan_dev_configure(vxlan->net, dev, &conf, true);
> + err = vxlan_dev_configure(vxlan->net, dev, &conf, true, extack);
> if (err)
> return err;
>
> @@ -3592,7 +3638,7 @@ struct net_device *vxlan_dev_create(struct net *net, const char *name,
> if (IS_ERR(dev))
> return dev;
>
> - err = __vxlan_dev_create(net, dev, conf);
> + err = __vxlan_dev_create(net, dev, conf, NULL);
> if (err < 0) {
> free_netdev(dev);
> return ERR_PTR(err);
> --
> 1.8.3.1
>
^ permalink raw reply
* [PATCH] fm10k: Fix misuse of net_ratelimit()
From: Joe Perches @ 2017-08-11 16:17 UTC (permalink / raw)
To: Jeff Kirsher; +Cc: intel-wired-lan, netdev, linux-kernel
Correct the backward logic using !net_ratelimit()
Miscellanea:
o Add a blank line before the error return label
Signed-off-by: Joe Perches <joe@perches.com>
---
drivers/net/ethernet/intel/fm10k/fm10k_main.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_main.c b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
index 9dffaba85ae6..489d8032e2f2 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_main.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
@@ -806,9 +806,10 @@ static int fm10k_tso(struct fm10k_ring *tx_ring,
tx_desc->mss = cpu_to_le16(skb_shinfo(skb)->gso_size);
return 1;
+
err_vxlan:
tx_ring->netdev->features &= ~NETIF_F_GSO_UDP_TUNNEL;
- if (!net_ratelimit())
+ if (net_ratelimit())
netdev_err(tx_ring->netdev,
"TSO requested for unsupported tunnel, disabling offload\n");
return -1;
--
2.10.0.rc2.1.g053435c
^ permalink raw reply related
* Re: [PATCH] openvswitch: Remove unnecessary newlines from OVS_NLERR uses
From: Joe Stringer @ 2017-08-11 16:16 UTC (permalink / raw)
To: Joe Perches; +Cc: Pravin Shelar, David S. Miller, netdev, ovs dev, LKML
In-Reply-To: <72f936a314a54a7957afdeb19e3d019c8c1dabea.1502450609.git.joe@perches.com>
On 11 August 2017 at 04:26, Joe Perches <joe@perches.com> wrote:
> OVS_NLERR already adds a newline so these just add blank
> lines to the logging.
>
> Signed-off-by: Joe Perches <joe@perches.com>
Acked-by: Joe Stringer <joe@ovn.org>
^ permalink raw reply
* Dear Recipient
From: Mr. Bernard Arnault @ 2017-08-11 15:37 UTC (permalink / raw)
To: netdev
You have been chosen to benefit from Mr. Bernard Arnault Philanthropic
Donation.Reply for more info via Email: firstchapter@surlapetitecote.com
Regards,
Mr. Bernard Arnault
---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus
^ permalink raw reply
* KASAN: slab-out-of-bounds from net_namespace.c:ops_init
From: Dave Jones @ 2017-08-11 16:10 UTC (permalink / raw)
To: netdev
==================================================================
BUG: KASAN: slab-out-of-bounds in ops_init+0x201/0x330
Write of size 8 at addr ffff88045744c448 by task trinity-c4/1499
CPU: 2 PID: 1499 Comm: trinity-c4 Not tainted 4.13.0-rc4-think+ #5
Call Trace:
dump_stack+0xc5/0x151
? dma_virt_map_sg+0xff/0xff
? show_regs_print_info+0x41/0x41
print_address_description+0xd9/0x260
kasan_report+0x27a/0x370
? ops_init+0x201/0x330
__asan_store8+0x57/0x90
ops_init+0x201/0x330
? net_alloc_generic+0x50/0x50
? __raw_spin_lock_init+0x21/0x80
? trace_hardirqs_on_caller+0x182/0x260
? lockdep_init_map+0xb2/0x2b0
setup_net+0x208/0x400
? ops_init+0x330/0x330
? copy_net_ns+0x151/0x390
? can_nice.part.81+0x20/0x20
? rcu_is_watching+0x8d/0xd0
? __lock_is_held+0x30/0xd0
? rcutorture_record_progress+0x20/0x20
? copy_net_ns+0x151/0x390
copy_net_ns+0x200/0x390
? net_drop_ns+0x20/0x20
? do_mount+0x19d0/0x19d0
? create_new_namespaces+0x97/0x450
? rcu_read_lock_sched_held+0x96/0xa0
? kmem_cache_alloc+0x28a/0x2f0
create_new_namespaces+0x317/0x450
? sys_ni_syscall+0x20/0x20
? cap_capable+0x7f/0xf0
unshare_nsproxy_namespaces+0x77/0xf0
SyS_unshare+0x573/0xbb0
? walk_process_tree+0x2a0/0x2a0
? lock_release+0x920/0x920
? lock_release+0x920/0x920
? mntput_no_expire+0x117/0x620
? rcu_is_watching+0x8d/0xd0
? exit_to_usermode_loop+0x1b0/0x1b0
? rcu_read_lock_sched_held+0x96/0xa0
? __context_tracking_exit.part.5+0x23d/0x2a0
? cpumask_check.part.2+0x10/0x10
? context_tracking_user_exit+0x30/0x30
? __f_unlock_pos+0x15/0x20
? SyS_read+0x146/0x160
? do_syscall_64+0xc0/0x3e0
? walk_process_tree+0x2a0/0x2a0
do_syscall_64+0x1bc/0x3e0
? syscall_return_slowpath+0x240/0x240
? mark_held_locks+0x23/0xb0
? return_from_SYSCALL_64+0x2d/0x7a
? trace_hardirqs_on_caller+0x182/0x260
? trace_hardirqs_on_thunk+0x1a/0x1c
entry_SYSCALL64_slow_path+0x25/0x25
RIP: 0033:0x7f9e1c454219
RSP: 002b:00007fff180f9c88 EFLAGS: 00000246
ORIG_RAX: 0000000000000110
RAX: ffffffffffffffda RBX: 0000000000000110 RCX: 00007f9e1c454219
RDX: 00000000000000c4 RSI: ffff8000000ff000 RDI: 0000000074060700
RBP: 00007fff180f9d30 R08: 0000000000000002 R09: 2fa420810090095e
R10: ffff880ffffffb40 R11: 0000000000000246 R12: 0000000000000002
R13: 00007f9e1cb06058 R14: 00007f9e1cb29698 R15: 00007f9e1cb06000
Allocated by task 1499:
save_stack_trace+0x1b/0x20
save_stack+0x43/0xd0
kasan_kmalloc+0xad/0xe0
__kmalloc+0x14b/0x370
net_alloc_generic+0x25/0x50
copy_net_ns+0x130/0x390
create_new_namespaces+0x317/0x450
unshare_nsproxy_namespaces+0x77/0xf0
SyS_unshare+0x573/0xbb0
do_syscall_64+0x1bc/0x3e0
return_from_SYSCALL_64+0x0/0x7a
Freed by task 504:
save_stack_trace+0x1b/0x20
save_stack+0x43/0xd0
kasan_slab_free+0x72/0xc0
kfree+0xe1/0x2f0
rcu_process_callbacks+0x5a6/0x1dc0
__do_softirq+0x1e7/0x817
The buggy address belongs to the object at ffff88045744c3c8
which belongs to the cache kmalloc-128 of size 128
The buggy address is located 0 bytes to the right of
128-byte region [ffff88045744c3c8, ffff88045744c448)
The buggy address belongs to the page:
page:ffffea00115d1300 count:1 mapcount:0 mapping: (null) index:0x0
compound_mapcount: 0
flags: 0x8000000000008100(slab|head)
raw: 8000000000008100 0000000000000000 0000000000000000 0000000100110011
raw: ffffea00113f2b20 ffffea0011328a20 ffff880467c0f140 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff88045744c300: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
ffff88045744c380: fc fc fc fc fc fc fc fc fc 00 00 00 00 00 00 00
>ffff88045744c400: 00 00 00 00 00 00 00 00 00 fc fc fc fc fc fc fc
^
ffff88045744c480: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
ffff88045744c500: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
==================================================================
^ permalink raw reply
* Re: [PATCH net-next v2] vxlan: change vxlan_[config_]validate() to use netlink_ext_ack for error reporting
From: Jiri Benc @ 2017-08-11 15:47 UTC (permalink / raw)
To: Girish Moodalbail; +Cc: pshelar, davem, netdev, mschiffer, roopa
In-Reply-To: <1502399795-25660-1-git-send-email-girish.moodalbail@oracle.com>
On Thu, 10 Aug 2017 14:16:35 -0700, Girish Moodalbail wrote:
> + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_PORT_RANGE],
> + "Provided source port range bounds is invalid");
s/bounds//?
> + if (conf->label && !use_ipv6) {
> + NL_SET_ERR_MSG(extack,
> + "Label attribute only applies for IPv6 VXLAN devices");
applies to
> + if (!lowerdev) {
> + NL_SET_ERR_MSG(extack,
> + "Specified interface for tunnel endpoint communications not found");
s/communications//?
> + if (vxlan_addr_multicast(&conf->remote_ip)) {
> + NL_SET_ERR_MSG(extack,
> + "Interface need to be specified for multicast destination");
needs
> + if (conf->flags & VXLAN_F_IPV6_LINKLOCAL) {
> + NL_SET_ERR_MSG(extack,
> + "Interface need to be specified for link-local local/remote addresses");
needs
Looks good to me otherwise, thanks!
Jiri
^ permalink raw reply
* Re: [PATCH] mlxsw: make mlxsw_config_profile const
From: Jiri Pirko @ 2017-08-11 15:32 UTC (permalink / raw)
To: Bhumika Goyal; +Cc: julia.lawall, jiri, idosch, netdev, linux-kernel
In-Reply-To: <1502458842-31516-1-git-send-email-bhumirks@gmail.com>
Fri, Aug 11, 2017 at 03:40:42PM CEST, bhumirks@gmail.com wrote:
>Make these structures const as they only stored in the profile field of
>a mlxsw_driver structure, which is of type const.
>Done using Coccinelle.
>
>Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
^ permalink raw reply
* Re: [PATCH 2/2] drivers: add vpls support
From: Roopa Prabhu @ 2017-08-11 15:14 UTC (permalink / raw)
To: David Lamparter; +Cc: Amine Kherbouche, netdev@vger.kernel.org
In-Reply-To: <20170811125547.GZ773745@eidolon>
On Fri, Aug 11, 2017 at 5:55 AM, David Lamparter <equinox@diac24.net> wrote:
> On Thu, Aug 10, 2017 at 10:28:37PM +0200, Amine Kherbouche wrote:
>> This commit introduces the support of VPLS virtual device, that allows
>> performing L2VPN multipoint to multipoint communication over MPLS PSN.
>>
>> VPLS device encap received ethernet frame over mpls packet and send it the
>> output device, in the other direction, when receiving the right configured
>> mpls packet, the matched mpls route calls the handler vpls function,
>> then pulls out the mpls header and send it back the entry point via
>> netif_rx().
>>
>> Two functions, mpls_entry_encode() and mpls_output_possible() are
>> exported from mpls/internal.h to be able to use them inside vpls driver.
>>
>> Signed-off-by: Amine Kherbouche <amine.kherbouche@6wind.com>
>
[snip]
> [...]
>> +union vpls_nh {
>> + struct in6_addr addr6;
>> + struct in_addr addr;
>> +};
>> +
>> +struct vpls_dst {
>> + struct net_device *dev;
>> + union vpls_nh addr;
>> + u32 label_in, label_out;
>> + u32 id;
>> + u16 vlan_id;
>
> I looked at VLAN support and decided against it because the bridge layer
> can handle this perfectly fine by using the bridge's vlan support to tag
> a port's pvid.
yes, agreed. there is no need for vlan here. The bridge can be
configured with the required vlan
mapping on the vpls port.
>
>> + u8 via_table;
>> + u8 flags;
>> + u8 ttl;
>> +};
>
> [...]
>> +struct vpls_priv {
>> + struct net *encap_net;
>> + struct vpls_dst dst;
>> +};
>> +
>> +static struct nla_policy vpls_policy[IFLA_VPLS_MAX + 1] = {
>> + [IFLA_VPLS_ID] = { .type = NLA_U32 },
>> + [IFLA_VPLS_IN_LABEL] = { .type = NLA_U32 },
>> + [IFLA_VPLS_OUT_LABEL] = { .type = NLA_U32 },
>> + [IFLA_VPLS_OIF] = { .type = NLA_U32 },
>> + [IFLA_VPLS_TTL] = { .type = NLA_U8 },
>> + [IFLA_VPLS_VLANID] = { .type = NLA_U8 },
>> + [IFLA_VPLS_NH] = { .type = NLA_U32 },
>> + [IFLA_VPLS_NH6] = { .len = sizeof(struct in6_addr) },
>> +};
>
> The original patchset was point-to-multipoint in a single netdev, and
> had some starts on optimized multicast support (which, admittedly, is a
> bit of a fringe thing, but still.)
>
I had been thinking about this as a single netdevice as well...which
can work with
the bridge driver using per vlan dst_metadata infra (similar to vxlan
single device and per vlan - vxlan mapping).
Multiple netdevice with one per vlan-vpls-id will work as well... but
starting with a single netdev
will be better (helps with scaling).
^ permalink raw reply
* Re: [PATCH 3/3] tls: Use correct sk->sk_prot for IPV6
From: kbuild test robot @ 2017-08-11 15:06 UTC (permalink / raw)
To: Ilya Lesokhin
Cc: kbuild-all, netdev, davejwatson, aviadye, Ilya Lesokhin,
Boris Pismenny
In-Reply-To: <1502361196-127278-4-git-send-email-ilyal@mellanox.com>
[-- Attachment #1: Type: text/plain, Size: 3248 bytes --]
Hi Ilya,
[auto build test ERROR on net/master]
[also build test ERROR on v4.13-rc4 next-20170811]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Ilya-Lesokhin/ipv6-Prevent-unexpected-sk-sk_prot-changes/20170811-223050
config: i386-randconfig-x018-201732 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
net/tls/tls_main.c: In function 'do_tls_setsockopt_tx':
>> net/tls/tls_main.c:362:43: error: 'TLSV6' undeclared (first use in this function)
int ip_ver = sk->sk_family == AF_INET6 ? TLSV6 : TLSV4;
^~~~~
net/tls/tls_main.c:362:43: note: each undeclared identifier is reported only once for each function it appears in
vim +/TLSV6 +362 net/tls/tls_main.c
354
355 static int do_tls_setsockopt_tx(struct sock *sk, char __user *optval,
356 unsigned int optlen)
357 {
358 struct tls_crypto_info *crypto_info, tmp_crypto_info;
359 struct tls_context *ctx = tls_get_ctx(sk);
360 struct proto *prot = NULL;
361 int rc = 0;
> 362 int ip_ver = sk->sk_family == AF_INET6 ? TLSV6 : TLSV4;
363
364 if (!optval || (optlen < sizeof(*crypto_info))) {
365 rc = -EINVAL;
366 goto out;
367 }
368
369 rc = copy_from_user(&tmp_crypto_info, optval, sizeof(*crypto_info));
370 if (rc) {
371 rc = -EFAULT;
372 goto out;
373 }
374
375 /* check version */
376 if (tmp_crypto_info.version != TLS_1_2_VERSION) {
377 rc = -ENOTSUPP;
378 goto out;
379 }
380
381 /* get user crypto info */
382 crypto_info = &ctx->crypto_send;
383
384 /* Currently we don't support set crypto info more than one time */
385 if (TLS_CRYPTO_INFO_READY(crypto_info))
386 goto out;
387
388 switch (tmp_crypto_info.cipher_type) {
389 case TLS_CIPHER_AES_GCM_128: {
390 if (optlen != sizeof(struct tls12_crypto_info_aes_gcm_128)) {
391 rc = -EINVAL;
392 goto out;
393 }
394 rc = copy_from_user(
395 crypto_info,
396 optval,
397 sizeof(struct tls12_crypto_info_aes_gcm_128));
398
399 if (rc) {
400 rc = -EFAULT;
401 goto err_crypto_info;
402 }
403 break;
404 }
405 default:
406 rc = -EINVAL;
407 goto out;
408 }
409
410 ctx->sk_write_space = sk->sk_write_space;
411 sk->sk_write_space = tls_write_space;
412
413 ctx->sk_proto_close = sk->sk_prot->close;
414
415 /* currently SW is default, we will have ethtool in future */
416 rc = tls_set_sw_offload(sk, ctx);
417 prot = &tls_prots[ip_ver][TLS_SW_TX];
418 if (rc)
419 goto err_crypto_info;
420
421 sk->sk_prot = prot;
422 goto out;
423
424 err_crypto_info:
425 memset(crypto_info, 0, sizeof(*crypto_info));
426 out:
427 return rc;
428 }
429
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 33510 bytes --]
^ permalink raw reply
* Re: [PATCH 2/3] ARM: sun8i: sunxi-h3-h5: add phy-is-integrated property to internal PHY
From: Florian Fainelli @ 2017-08-11 15:03 UTC (permalink / raw)
To: Corentin Labbe, Chen-Yu Tsai, robh+dt, mark.rutland
Cc: Russell King, Maxime Ripard, Giuseppe Cavallaro, alexandre.torgue,
netdev, devicetree, linux-kernel, linux-arm-kernel, andrew
In-Reply-To: <20170811132526.GD25147@Red>
On August 11, 2017 6:25:26 AM PDT, Corentin Labbe <clabbe.montjoie@gmail.com> wrote:
>On Fri, Aug 11, 2017 at 04:22:11PM +0800, Chen-Yu Tsai wrote:
>> On Fri, Aug 11, 2017 at 4:19 PM, Corentin Labbe
>> <clabbe.montjoie@gmail.com> wrote:
>> > On Fri, Aug 11, 2017 at 04:11:13PM +0800, Chen-Yu Tsai wrote:
>> >> On Fri, Aug 11, 2017 at 4:05 PM, Corentin Labbe
>> >> <clabbe.montjoie@gmail.com> wrote:
>> >> > On Fri, Aug 11, 2017 at 10:42:51AM +0800, Chen-Yu Tsai wrote:
>> >> >> Hi,
>> >> >>
>> >> >> On Thu, Aug 10, 2017 at 4:51 PM, Corentin Labbe
>> >> >> <clabbe.montjoie@gmail.com> wrote:
>> >> >> > This patch add the new phy-is-integrated property to the
>internal PHY
>> >> >> > node.
>> >> >> >
>> >> >> > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
>> >> >> > ---
>> >> >> > arch/arm/boot/dts/sunxi-h3-h5.dtsi | 1 +
>> >> >> > 1 file changed, 1 insertion(+)
>> >> >> >
>> >> >> > diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
>b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
>> >> >> > index 4b599b5d26f6..54fc24e4c569 100644
>> >> >> > --- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
>> >> >> > +++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
>> >> >> > @@ -425,6 +425,7 @@
>> >> >> > reg = <1>;
>> >> >> > clocks = <&ccu
>CLK_BUS_EPHY>;
>> >> >> > resets = <&ccu
>RST_BUS_EPHY>;
>> >> >> > + phy-is-integrated;
>> >> >>
>> >> >> You also need to "delete" this property at the board level for
>> >> >> any board that has the external PHY at address <1>. Otherwise
>> >> >> they will stop working. This is due to the internal and
>external
>> >> >> PHYs having the same path and node name in the device tree, so
>> >> >> they are effectively the same node.
>> >> >>
>> >> >> ChenYu
>> >> >>
>> >> >
>> >> > They have not the same name, ext_rgmii_phy vs int_mii_phy.
>> >>
>> >> That is just the label. The label plays no part in device tree
>merging. The path
>> >>
>> >> /soc/ethernet@1c30000/mdio/ethernet-phy@1
>> >>
>> >> is the same. You can look under
>> >>
>> >> /proc/device-tree/soc/ethernet@1c30000/mdio
>> >>
>> >> on the OrangePI Plus 2E or any other H3 board that uses an
>> >> external PHY at address 1.
>> >>
>> >> ChenYu
>> >
>> > Since we get the phy node by phy-handle and not by path, I think
>all should be good.
>>
>> You are not getting me. The fact that the two seemingly separate
>> nodes are merged together means, whatever properties you put in
>> the internal PHY node, also affect the external PHY node. Once
>> compiled, they are the SAME node.
>
>Hello Rob, florian, mark
>
>Adding a delete property on all external ethernet-phy@1 is a bit
>overkill, and I dont like the idea that nodes are merged.
This is not exactly up to you that's just how DTC works.
>What do you think about other possible solutions:
>- Using integrated-phy@1 for the integrated PHY node name
That might be okay although you are using now a seemingly non-standard unit name.
>- Using a fake address like 31 (see patch below)
You could also drop the address part in the unit name although we'd probably get a DTC warning for that.
I suspect both of your solutions and what I mentioned above will be producing DTC warnings to some extent... Rob what do you think?
>
>If you have any other solution...
>
>Regards
>
>From fe39183946f7f4a6e21bce38fd8e4c1413012d68 Mon Sep 17 00:00:00 2001
>From: Corentin Labbe <clabbe.montjoie@gmail.com>
>Date: Fri, 11 Aug 2017 14:49:54 +0200
>Subject: [PATCH] ARM: sun8i: sunxi-h3-h5: Prevent merge of external and
> integrated PHY
>
>Actually, some external and integrated PHY are merged due to same
>dtnode
>name "ethernet-phy@1".
>
>This is problematic when we will want to use the phy-is-integrated
>property. (Need to delete it on all external PHY node)
>
>An easy solution is to set integrated PHY nodeaddresss at a fake one
>that would never be used.
>Since board makers currently only provides PHY at addresses 1 and 7,
>we will use 31.
>
>Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
>---
> arch/arm/boot/dts/sunxi-h3-h5.dtsi | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
>diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
>b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
>index 54fc24e4c569..2110b0069e33 100644
>--- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
>+++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
>@@ -420,7 +420,15 @@
> mdio: mdio {
> #address-cells = <1>;
> #size-cells = <0>;
>- int_mii_phy: ethernet-phy@1 {
>+ /*
>+ * Using 31 permits to make a separation between
>+ * this integrated PHY and external ones.
>+ * Without it, external "ethernet-phy@1" will be
>+ * merged with it (due to same dtnode name).
>+ * Board makers currently only provides PHY at
>+ * addresses 1 and 7.
>+ */
>+ int_mii_phy: ethernet-phy@31 {
> compatible = "ethernet-phy-ieee802.3-c22";
> reg = <1>;
> clocks = <&ccu CLK_BUS_EPHY>;
--
Florian
^ permalink raw reply
* Re: [PATCH 1/2] mpls: add handlers
From: Roopa Prabhu @ 2017-08-11 14:37 UTC (permalink / raw)
To: David Lamparter; +Cc: Amine Kherbouche, netdev@vger.kernel.org
In-Reply-To: <20170811123405.GY773745@eidolon>
On Fri, Aug 11, 2017 at 5:34 AM, David Lamparter <equinox@diac24.net> wrote:
> On Thu, Aug 10, 2017 at 10:28:36PM +0200, Amine Kherbouche wrote:
>> Mpls handler allows creation/deletion of mpls routes without using
>> rtnetlink. When an incoming mpls packet matches this route, the saved
>> function handler is called.
>
> Since I originally authored this patch, I have come to believe that it
> might be unneccessarily complicated. It is unlikely that a lot of
> different "handlers" will exist here; the only things I can think of
> are VPLS support and BIER-MPLS multicast replication. I'm not saying
> it's a bad idea, but, well, this was in the README that I gave to 6WIND
> with this code:
>
> ...
yes, I would also prefer just exporting the functions and calling
them directly instead of adding a
handler layer. We can move to that later if it becomes necessary.
^ permalink raw reply
* Re: After a while of system running no incoming UDP any more?
From: Marc Haber @ 2017-08-11 14:34 UTC (permalink / raw)
To: Paolo Abeni, netdev
In-Reply-To: <20170728121434.vld76q7kohzpmtk2@torres.zugschlus.de>
On Fri, Jul 28, 2017 at 02:14:34PM +0200, Marc Haber wrote:
> I can confirm that these two changes make a system in bad state work
> again immediately. Will try the patch on 4.12.4 later today.
After upgrading my test systems to 4.12.5, the issue reappeared. This
shows me that the patch indeed helped (my patched 4.12.4 kernels didn't
show the bad behavior), and that the patch didn't make its way into
4.12.5. The patch applied to 4.12.5, kernels are building.
The run-time fix works as well.
Greetings
Marc
--
-----------------------------------------------------------------------------
Marc Haber | "I don't trust Computers. They | Mailadresse im Header
Leimen, Germany | lose things." Winona Ryder | Fon: *49 6224 1600402
Nordisch by Nature | How to make an American Quilt | Fax: *49 6224 1600421
^ permalink raw reply
* Re: Driver profiles RFC
From: Roopa Prabhu @ 2017-08-11 14:34 UTC (permalink / raw)
To: Arkadi Sharshevsky
Cc: netdev@vger.kernel.org, David Miller, ivecera, Florian Fainelli,
Vivien Didelot, John Fastabend, Andrew Lunn, Jiri Pirko, mlxsw
In-Reply-To: <8c2cd3d2-70a8-b583-28af-6c0b1b6f82c0@mellanox.com>
On Wed, Aug 9, 2017 at 4:43 AM, Arkadi Sharshevsky <arkadis@mellanox.com> wrote:
>
>
> On 08/08/2017 07:08 PM, Roopa Prabhu wrote:
>> On Tue, Aug 8, 2017 at 6:15 AM, Arkadi Sharshevsky <arkadis@mellanox.com> wrote:
>>>
[snip]
>>> User Configuration
>>> ------------------
>>> Such an UAPI is very low level, and thus an average user may not know how to
>>> adjust this sizes according to his needs. The vendor can provide several
>>> tested configuration files that the user can choose from. Each config file
>>> will be measured in terms of: MAC addresses, L3 Neighbors (IPv4, IPv6),
>>> LPM entries (IPv4,IPv6) in order to provide approximate results. By this an
>>> average user will choose one of the provided ones. Furthermore, a more
>>> advanced user could play with the numbers for his personal benefit.
>>>
>>> Reference
>>> =========
>>> [1] https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fnetdevconf.org%2F2.1%2Fpapers%2Fdpipe_netdev_2_1.odt&data=02%7C01%7Carkadis%40mellanox.com%7Cc64b0d54e3e94d07b64c08d4de77bf8b%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636378053281241266&sdata=9u%2BFwGF%2FjkmNogPF7Cm%2FfwJsaPVkr%2BC3%2F8x1IVbszRg%3D&reserved=0
>>>
>>
>> Thanks for sending this out. There is very much a need for this.
>> and agree, user-space app config can translate to what values they want and
>> kernel api can be a low level api.
>>
>> But how about we align these resource limits with the kernel resource limits ?
>> For example we usually map l3 hw neighbor limits to kernel software gc_thresh
>> values (which are configurable via sysctl). This is one way to give
>> user immediate
>> feedback on resource full errors. It would be nice if we can introduce
>> limits for routes and
>> mac addresses. Defaults could be what they are today but user
>> configurable ...like I said,
>> neighbor subsystem already allows this.
>>
>
> Hi Roopa, thanks for the feedback.
>
> Regarding aligning the hardware tables sizes with the kernel software
> limits. The hardware resources (internal memory) are much more limited
> than the software one. Please consider the following scenario:
>
> 1. User adds limit to neighbor table (as you suggested), which uses the
> hash memory portion.
> 2. User adds many routes, the routes uses the hash memory resource as well,
> potentially.
> 3. The kernel adds some neighbors dynamically, the neighbor offloading
> fails due to lack of this shared resource, the user get confused because
> its lower then what he configured in 1).
>
> Thus providing max size on specific table is not well defined due to
> limited
> shared resource. Thus, the feedback the user gets can be not very accurate.
> Furthermore, guessing the resource partitioning based only on the subset of
> tables which use it makes me little bit uncomfortable.
yep, understood. I am aware of some of these problems as well.
>
> The proposed API aims at solving this issue by providing abstraction for
> this hw behavior, and provide the connection with the hardware table, thus
> providing more accurate and well defined description of the system.
>
> I totally agree that this API should be enhanced in order provide the
> occupancy of the this 'resource'. For example, the user first observe the
> tables and sees the resource<->table mapping, then see the resource
> occupancy:
>
> #devlink dpipe resource occupancy pci/0000:03:00.0 Mem
>
> By this the user can understand the offloading limitation, and maybe figure
> out that he should change the partitioning.
>
yes, sounds good.
^ permalink raw reply
* Re: [PATCH] mISDN: Fix null pointer dereference at mISDN_FsmNew
From: isdn @ 2017-08-11 14:31 UTC (permalink / raw)
To: Anton Vasilyev
Cc: Geliang Tang, David S. Miller, Johannes Berg, Stephen Hemminger,
netdev, linux-kernel, ldv-project
In-Reply-To: <1502456242-1695-1-git-send-email-vasilyev@ispras.ru>
Hi Anton,
good spot, thanks. Dave please apply.
Karsten
Am 11.08.2017 um 14:57 schrieb Anton Vasilyev:
> If mISDN_FsmNew() fails to allocate memory for jumpmatrix
> then null pointer dereference will occur on any write to
> jumpmatrix.
>
> The patch adds check on successful allocation and
> corresponding error handling.
>
> Found by Linux Driver Verification project (linuxtesting.org).
>
> Signed-off-by: Anton Vasilyev <vasilyev@ispras.ru>
> ---
> drivers/isdn/mISDN/fsm.c | 5 ++++-
> drivers/isdn/mISDN/fsm.h | 2 +-
> drivers/isdn/mISDN/layer1.c | 3 +--
> drivers/isdn/mISDN/layer2.c | 15 +++++++++++++--
> drivers/isdn/mISDN/tei.c | 20 +++++++++++++++++---
> 5 files changed, 36 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/isdn/mISDN/fsm.c b/drivers/isdn/mISDN/fsm.c
> index 78fc5d5..92e6570 100644
> --- a/drivers/isdn/mISDN/fsm.c
> +++ b/drivers/isdn/mISDN/fsm.c
> @@ -26,7 +26,7 @@
>
> #define FSM_TIMER_DEBUG 0
>
> -void
> +int
> mISDN_FsmNew(struct Fsm *fsm,
> struct FsmNode *fnlist, int fncount)
> {
> @@ -34,6 +34,8 @@ mISDN_FsmNew(struct Fsm *fsm,
>
> fsm->jumpmatrix = kzalloc(sizeof(FSMFNPTR) * fsm->state_count *
> fsm->event_count, GFP_KERNEL);
> + if (fsm->jumpmatrix == NULL)
> + return -ENOMEM;
>
> for (i = 0; i < fncount; i++)
> if ((fnlist[i].state >= fsm->state_count) ||
> @@ -45,6 +47,7 @@ mISDN_FsmNew(struct Fsm *fsm,
> } else
> fsm->jumpmatrix[fsm->state_count * fnlist[i].event +
> fnlist[i].state] = (FSMFNPTR) fnlist[i].routine;
> + return 0;
> }
> EXPORT_SYMBOL(mISDN_FsmNew);
>
> diff --git a/drivers/isdn/mISDN/fsm.h b/drivers/isdn/mISDN/fsm.h
> index 928f5be..e1def84 100644
> --- a/drivers/isdn/mISDN/fsm.h
> +++ b/drivers/isdn/mISDN/fsm.h
> @@ -55,7 +55,7 @@ struct FsmTimer {
> void *arg;
> };
>
> -extern void -(struct Fsm *, struct FsmNode *, int);
> +extern int mISDN_FsmNew(struct Fsm *, struct FsmNode *, int);
> extern void mISDN_FsmFree(struct Fsm *);
> extern int mISDN_
(struct FsmInst *, int , void *);
> extern void mISDN_FsmChangeState(struct FsmInst *, int);
> diff --git a/drivers/isdn/mISDN/layer1.c b/drivers/isdn/mISDN/layer1.c
> index bebc57b..3192b0e 100644
> --- a/drivers/isdn/mISDN/layer1.c
> +++ b/drivers/isdn/mISDN/layer1.c
> @@ -414,8 +414,7 @@ l1_init(u_int *deb)
> l1fsm_s.event_count = L1_EVENT_COUNT;
> l1fsm_s.strEvent = strL1Event;
> l1fsm_s.strState = strL1SState;
> - mISDN_FsmNew(&l1fsm_s, L1SFnList, ARRAY_SIZE(L1SFnList));
> - return 0;
> + return mISDN_FsmNew(&l1fsm_s, L1SFnList, ARRAY_SIZE(L1SFnList));
> }
>
> void
> diff --git a/drivers/isdn/mISDN/layer2.c b/drivers/isdn/mISDN/layer2.c
> index 7243a67..9ff0903 100644
> --- a/drivers/isdn/mISDN/layer2.c
> +++ b/drivers/isdn/mISDN/layer2.c
> @@ -2247,15 +2247,26 @@ static struct Bprotocol X75SLP = {
> int
> Isdnl2_Init(u_int *deb)
> {
> + int res;
> debug = deb;
> mISDN_register_Bprotocol(&X75SLP);
> l2fsm.state_count = L2_STATE_COUNT;
> l2fsm.event_count = L2_EVENT_COUNT;
> l2fsm.strEvent = strL2Event;
> l2fsm.strState = strL2State;
> - mISDN_FsmNew(&l2fsm, L2FnList, ARRAY_SIZE(L2FnList));
> - TEIInit(deb);
> + res = mISDN_FsmNew(&l2fsm, L2FnList, ARRAY_SIZE(L2FnList));
> + if (res)
> + goto error;
> + res = TEIInit(deb);
> + if (res)
> + goto error_fsm;
> return 0;
> +
> +error_fsm:
> + mISDN_FsmFree(&l2fsm);
> +error:
> + mISDN_unregister_Bprotocol(&X75SLP);
> + return res;
> }
>
> void
> diff --git a/drivers/isdn/mISDN/tei.c b/drivers/isdn/mISDN/tei.c
> index 908127e..12d9e5f 100644
> --- a/drivers/isdn/mISDN/tei.c
> +++ b/drivers/isdn/mISDN/tei.c
> @@ -1387,23 +1387,37 @@ create_teimanager(struct mISDNdevice *dev)
>
> int TEIInit(u_int *deb)
> {
> + int res;
> debug = deb;
> teifsmu.state_count = TEI_STATE_COUNT;
> teifsmu.event_count = TEI_EVENT_COUNT;
> teifsmu.strEvent = strTeiEvent;
> teifsmu.strState = strTeiState;
> - mISDN_FsmNew(&teifsmu, TeiFnListUser, ARRAY_SIZE(TeiFnListUser));
> + res = mISDN_FsmNew(&teifsmu, TeiFnListUser, ARRAY_SIZE(TeiFnListUser));
> + if (res)
> + goto error;
> teifsmn.state_count = TEI_STATE_COUNT;
> teifsmn.event_count = TEI_EVENT_COUNT;
> teifsmn.strEvent = strTeiEvent;
> teifsmn.strState = strTeiState;
> - mISDN_FsmNew(&teifsmn, TeiFnListNet, ARRAY_SIZE(TeiFnListNet));
> + res = mISDN_FsmNew(&teifsmn, TeiFnListNet, ARRAY_SIZE(TeiFnListNet));
> + if (res)
> + goto error_smn;
> deactfsm.state_count = DEACT_STATE_COUNT;
> deactfsm.event_count = DEACT_EVENT_COUNT;
> deactfsm.strEvent = strDeactEvent;
> deactfsm.strState = strDeactState;
> - mISDN_FsmNew(&deactfsm, DeactFnList, ARRAY_SIZE(DeactFnList));
> + res = mISDN_FsmNew(&deactfsm, DeactFnList, ARRAY_SIZE(DeactFnList));
> + if (res)
> + goto error_deact;
> return 0;
> +
> +error_deact:
> + mISDN_FsmFree(&teifsmn);
> +error_smn:
> + mISDN_FsmFree(&teifsmu);
> +error:
> + return res;
> }
>
> void TEIFree(void)
>
^ permalink raw reply
* Re: [v2,02/20] ath6kl: constify usb_device_id
From: Kalle Valo @ 2017-08-11 13:59 UTC (permalink / raw)
To: Arvind Yadav; +Cc: kvalo, petkan, linux-kernel, netdev, linux-wireless
In-Reply-To: <1502295823-20824-3-git-send-email-arvind.yadav.cs@gmail.com>
Arvind Yadav <arvind.yadav.cs@gmail.com> wrote:
> usb_device_id are not supposed to change at runtime. All functions
> working with usb_device_id provided by <linux/usb.h> work with
> const usb_device_id. So mark the non-const structs as const.
>
> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
> Reviewed-by: Steve deRosier <derosier@gmail.com>
> Tested-by: Steve deRosier <derosier@gmail.com>
> Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>
2 patches applied to ath-next branch of ath.git, thanks.
e881a6584941 ath6kl: constify usb_device_id
76b07b30c46b ath9k: constify usb_device_id
--
https://patchwork.kernel.org/patch/9890987/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH net-next] gre: introduce native tunnel support for ERSPAN
From: kbuild test robot @ 2017-08-11 13:54 UTC (permalink / raw)
To: William Tu
Cc: kbuild-all, netdev, Meenakshi Vohra, Alexey Kuznetsov,
Hideaki YOSHIFUJI
In-Reply-To: <1502310157-7827-2-git-send-email-u9012063@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1297 bytes --]
Hi William,
[auto build test ERROR on net-next/master]
url: https://github.com/0day-ci/linux/commits/William-Tu/gre-introduce-native-tunnel-support-for-ERSPAN/20170811-210323
config: x86_64-allyesdebian (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
>> net//ipv4/ip_gre.c:51:24: fatal error: net/erspan.h: No such file or directory
#include <net/erspan.h>
^
compilation terminated.
vim +51 net//ipv4/ip_gre.c
35
36 #include <net/sock.h>
37 #include <net/ip.h>
38 #include <net/icmp.h>
39 #include <net/protocol.h>
40 #include <net/ip_tunnels.h>
41 #include <net/arp.h>
42 #include <net/checksum.h>
43 #include <net/dsfield.h>
44 #include <net/inet_ecn.h>
45 #include <net/xfrm.h>
46 #include <net/net_namespace.h>
47 #include <net/netns/generic.h>
48 #include <net/rtnetlink.h>
49 #include <net/gre.h>
50 #include <net/dst_metadata.h>
> 51 #include <net/erspan.h>
52
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 39255 bytes --]
^ permalink raw reply
* Re: [v3,06/20] brcm80211: constify usb_device_id
From: Kalle Valo @ 2017-08-11 13:54 UTC (permalink / raw)
To: Arvind Yadav
Cc: arend.vanspriel, franky.lin, hante.meuleman, chi-hsien.lin,
wsa-dev, linux-wireless, brcm80211-dev-list.pdl,
brcm80211-dev-list, netdev, linux-kernel
In-Reply-To: <ff83342cc9413d79ba18e05b2cb0b53bff338b2f.1502428689.git.arvind.yadav.cs@gmail.com>
Arvind Yadav <arvind.yadav.cs@gmail.com> wrote:
> usb_device_id are not supposed to change at runtime. All functions
> working with usb_device_id provided by <linux/usb.h> work with
> const usb_device_id. So mark the non-const structs as const.
>
> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
Patch applied to wireless-drivers-next.git, thanks.
a67b133b9da5 brcm80211: constify usb_device_id
--
https://patchwork.kernel.org/patch/9894975/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* [PATCHv2 net-next] selftests: bpf: add check for ip XDP redirect
From: William Tu @ 2017-08-11 13:46 UTC (permalink / raw)
To: netdev; +Cc: John Fastabend
Kernel test robot reports error when running test_xdp_redirect.sh.
Check if ip tool supports xdpgeneric, if not, skip the test.
Signed-off-by: William Tu <u9012063@gmail.com>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
Cc: John Fastabend <john.fastabend@gmail.com>
---
tools/testing/selftests/bpf/test_xdp_redirect.sh | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/tools/testing/selftests/bpf/test_xdp_redirect.sh b/tools/testing/selftests/bpf/test_xdp_redirect.sh
index d8c73ed6e040..344a3656dea6 100755
--- a/tools/testing/selftests/bpf/test_xdp_redirect.sh
+++ b/tools/testing/selftests/bpf/test_xdp_redirect.sh
@@ -23,6 +23,11 @@ cleanup()
ip netns del ns2 2> /dev/null
}
+ip link set dev lo xdpgeneric off 2>/dev/null > /dev/null
+if [ $? -ne 0 ];then
+ echo "selftests: [SKIP] Could not run test without the ip xdpgeneric support"
+ exit 0
+fi
set -e
ip netns add ns1
--
2.7.4
^ permalink raw reply related
* [PATCH] mlxsw: make mlxsw_config_profile const
From: Bhumika Goyal @ 2017-08-11 13:40 UTC (permalink / raw)
To: julia.lawall, jiri, idosch, netdev, linux-kernel; +Cc: Bhumika Goyal
Make these structures const as they only stored in the profile field of
a mlxsw_driver structure, which is of type const.
Done using Coccinelle.
Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 2 +-
drivers/net/ethernet/mellanox/mlxsw/switchib.c | 2 +-
drivers/net/ethernet/mellanox/mlxsw/switchx2.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index eb7c454..82f8ccd 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -3743,7 +3743,7 @@ static void mlxsw_sp_fini(struct mlxsw_core *mlxsw_core)
mlxsw_sp_fids_fini(mlxsw_sp);
}
-static struct mlxsw_config_profile mlxsw_sp_config_profile = {
+static const struct mlxsw_config_profile mlxsw_sp_config_profile = {
.used_max_vepa_channels = 1,
.max_vepa_channels = 0,
.used_max_mid = 1,
diff --git a/drivers/net/ethernet/mellanox/mlxsw/switchib.c b/drivers/net/ethernet/mellanox/mlxsw/switchib.c
index 74341fe..ab7a298 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/switchib.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/switchib.c
@@ -497,7 +497,7 @@ static void mlxsw_sib_fini(struct mlxsw_core *mlxsw_core)
mlxsw_sib_ports_remove(mlxsw_sib);
}
-static struct mlxsw_config_profile mlxsw_sib_config_profile = {
+static const struct mlxsw_config_profile mlxsw_sib_config_profile = {
.used_max_system_port = 1,
.max_system_port = 48000,
.used_max_ib_mc = 1,
diff --git a/drivers/net/ethernet/mellanox/mlxsw/switchx2.c b/drivers/net/ethernet/mellanox/mlxsw/switchx2.c
index 3b0f724..f3c29bb 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/switchx2.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/switchx2.c
@@ -1674,7 +1674,7 @@ static void mlxsw_sx_fini(struct mlxsw_core *mlxsw_core)
mlxsw_sx_ports_remove(mlxsw_sx);
}
-static struct mlxsw_config_profile mlxsw_sx_config_profile = {
+static const struct mlxsw_config_profile mlxsw_sx_config_profile = {
.used_max_vepa_channels = 1,
.max_vepa_channels = 0,
.used_max_mid = 1,
--
1.9.1
^ permalink raw reply related
* Re: [PATCH] selftests: bpf: add check for ip XDP redirect
From: William Tu @ 2017-08-11 13:35 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: Linux Kernel Network Developers, John Fastabend
In-Reply-To: <598D9597.2030907@iogearbox.net>
thanks! I will fix it and re-submit.
William
On Fri, Aug 11, 2017 at 4:31 AM, Daniel Borkmann <daniel@iogearbox.net> wrote:
> On 08/11/2017 02:02 AM, William Tu wrote:
>>
>> Kernel test robot reports error when running test_xdp_redirect.sh.
>> Check if ip tool supports xdpgeneric, if not, skip the test.
>>
>> Signed-off-by: William Tu <u9012063@gmail.com>
>> Cc: Daniel Borkmann <daniel@iogearbox.net>
>> Cc: John Fastabend <john.fastabend@gmail.com>
>> ---
>> tools/testing/selftests/bpf/test_xdp_redirect.sh | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/tools/testing/selftests/bpf/test_xdp_redirect.sh
>> b/tools/testing/selftests/bpf/test_xdp_redirect.sh
>> index d8c73ed6e040..c5a8b7173c12 100755
>> --- a/tools/testing/selftests/bpf/test_xdp_redirect.sh
>> +++ b/tools/testing/selftests/bpf/test_xdp_redirect.sh
>> @@ -23,6 +23,11 @@ cleanup()
>> ip netns del ns2 2> /dev/null
>> }
>>
>> +ip link set dev lo xdpgeneric off 2>/dev/null > /dev/null
>> +if [ $? -ne 0 ];then
>> + echo "selftests: [SKIP] Could not run test without the ip
>> xdpgeneric support"
>> + exit 0
>
>
> Small nit: could you make that indent consistently as tabs
> like in your cleanup() function slightly further above?
>
> Other than that (+ subject should also be [PATCH net-next]):
>
> Acked-by: Daniel Borkmann <daniel@iogearbox.net>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox