* Re: [PATCH net-next v2 07/12] net: dsa: Add ability to program multicast filter for CPU port
From: Vivien Didelot @ 2019-01-30 22:28 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev, Florian Fainelli, andrew, davem, idosch, jiri,
ilias.apalodimas, ivan.khoronzhuk, roopa, nikolay
In-Reply-To: <20190130005548.2212-8-f.fainelli@gmail.com>
Hi Florian,
On Tue, 29 Jan 2019 16:55:43 -0800, Florian Fainelli <f.fainelli@gmail.com> wrote:
> +static int dsa_slave_sync_unsync_mdb_addr(struct net_device *dev,
> + const unsigned char *addr, bool add)
> +{
> + struct switchdev_obj_port_mdb mdb = {
> + .obj = {
> + .id = SWITCHDEV_OBJ_ID_HOST_MDB,
> + .flags = SWITCHDEV_F_DEFER,
> + },
> + .vid = 0,
> + };
> + int ret = -EOPNOTSUPP;
Assignment unneeded here.
> +
> + ether_addr_copy(mdb.addr, addr);
> + if (add)
> + ret = switchdev_port_obj_add(dev, &mdb.obj, NULL);
> + else
> + ret = switchdev_port_obj_del(dev, &mdb.obj);
> +
> + return ret;
> +}
> +
> +static int dsa_slave_sync_mdb_addr(struct net_device *dev,
> + const unsigned char *addr)
> +{
> + return dsa_slave_sync_unsync_mdb_addr(dev, addr, true);
> +}
> +
> +static int dsa_slave_unsync_mdb_addr(struct net_device *dev,
> + const unsigned char *addr)
> +{
> + return dsa_slave_sync_unsync_mdb_addr(dev, addr, false);
> +}
This wrapper isn't necessary IMO. I'd go with something like:
static int dsa_slave_sync(struct net_device *dev, const unsigned char *addr)
{
struct switchdev_obj_port_mdb mdb = {
.obj.id = SWITCHDEV_OBJ_ID_HOST_MDB,
.obj.flags = SWITCHDEV_F_DEFER,
};
ether_addr_copy(mdb.addr, addr);
return switchdev_port_obj_add(dev, &mdb.obj, NULL);
}
static int dsa_slave_unsync(struct net_device *dev, const unsigned char *addr)
{
struct switchdev_obj_port_mdb mdb = {
.obj.id = SWITCHDEV_OBJ_ID_HOST_MDB,
.obj.flags = SWITCHDEV_F_DEFER,
};
ether_addr_copy(mdb.addr, addr);
return switchdev_port_obj_del(dev, &mdb.obj);
}
We may eventually wrap this cryptic netdevery in:
static int dsa_slave_mc_sync(struct net_device *dev)
{
return __hw_addr_sync_dev(&dev->mc, dev, dsa_slave_sync, dsa_slave_unsync);
}
static void dsa_slave_mc_unsync(struct net_device *dev)
{
__hw_addr_unsync_dev(&dev->mc, dev, dsa_slave_sync);
}
> +
> static int dsa_slave_open(struct net_device *dev)
> {
> struct net_device *master = dsa_slave_to_master(dev);
> @@ -126,6 +159,8 @@ static int dsa_slave_close(struct net_device *dev)
>
> dev_mc_unsync(master, dev);
> dev_uc_unsync(master, dev);
> + __hw_addr_unsync_dev(&dev->mc, dev, dsa_slave_unsync_mdb_addr);
> +
> if (dev->flags & IFF_ALLMULTI)
> dev_set_allmulti(master, -1);
> if (dev->flags & IFF_PROMISC)
> @@ -150,7 +185,17 @@ static void dsa_slave_change_rx_flags(struct net_device *dev, int change)
> static void dsa_slave_set_rx_mode(struct net_device *dev)
> {
> struct net_device *master = dsa_slave_to_master(dev);
> + struct dsa_port *dp = dsa_slave_to_port(dev);
>
> + /* If the port is bridged, the bridge takes care of sending
> + * SWITCHDEV_OBJ_ID_HOST_MDB to program the host's MC filter
> + */
> + if (netdev_mc_empty(dev) || dp->bridge_dev)
> + goto out;
> +
> + __hw_addr_sync_dev(&dev->mc, dev, dsa_slave_sync_mdb_addr,
> + dsa_slave_unsync_mdb_addr);
And check the returned error code.
> +out:
> dev_mc_sync(master, dev);
> dev_uc_sync(master, dev);
> }
Thanks,
Vivien
^ permalink raw reply
* Re: [PATCH net-next v2 7/7] ethtool: add compat for devlink info
From: Jiri Pirko @ 2019-01-30 22:12 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, oss-drivers, andrew, f.fainelli, mkubecek, eugenem,
jonathan.lemon
In-Reply-To: <20190130190513.25718-8-jakub.kicinski@netronome.com>
Wed, Jan 30, 2019 at 08:05:13PM CET, jakub.kicinski@netronome.com wrote:
>If driver did not fill the fw_version field, try to call into
>the new devlink get_info op and collect the versions that way.
>We assume ethtool was always reporting running versions.
>
>Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
>---
> include/net/devlink.h | 7 ++++++
> net/core/devlink.c | 52 ++++++++++++++++++++++++++++++++++++++++++-
> net/core/ethtool.c | 7 ++++++
> 3 files changed, 65 insertions(+), 1 deletion(-)
>
>diff --git a/include/net/devlink.h b/include/net/devlink.h
>index c678ed0cb099..b4750e93303a 100644
>--- a/include/net/devlink.h
>+++ b/include/net/devlink.h
>@@ -640,6 +640,8 @@ int devlink_info_report_version(struct devlink_info_req *req,
> enum devlink_version_type type,
> const char *version_name,
> const char *version_value);
>+void devlink_compat_running_versions(struct net_device *dev,
>+ char *buf, size_t len);
>
> #else
>
>@@ -957,6 +959,11 @@ devlink_info_report_version(struct devlink_info_req *req,
> {
> return 0;
> }
>+
>+static inline void
>+devlink_compat_running_versions(struct net_device *dev, char *buf, size_t len)
>+{
>+}
> #endif
>
> #endif /* _NET_DEVLINK_H_ */
>diff --git a/net/core/devlink.c b/net/core/devlink.c
>index e2027d3a5e40..5313e5918ee2 100644
>--- a/net/core/devlink.c
>+++ b/net/core/devlink.c
>@@ -3715,12 +3715,18 @@ static int devlink_nl_cmd_region_read_dumpit(struct sk_buff *skb,
> }
>
> struct devlink_info_req {
>+ bool compat;
> struct sk_buff *msg;
>+ /* For compat call */
>+ char *buf;
union?
>+ size_t len;
> };
>
> int devlink_info_report_driver_name(struct devlink_info_req *req,
> const char *name)
> {
>+ if (req->compat)
>+ return 0;
> return nla_put_string(req->msg, DEVLINK_ATTR_INFO_DRV_NAME, name);
> }
> EXPORT_SYMBOL_GPL(devlink_info_report_driver_name);
>@@ -3728,6 +3734,8 @@ EXPORT_SYMBOL_GPL(devlink_info_report_driver_name);
> int devlink_info_report_serial_number(struct devlink_info_req *req,
> const char *sn)
> {
>+ if (req->compat)
>+ return 0;
> return nla_put_string(req->msg, DEVLINK_ATTR_INFO_SERIAL_NUMBER, sn);
> }
> EXPORT_SYMBOL_GPL(devlink_info_report_serial_number);
>@@ -3743,7 +3751,15 @@ int devlink_info_report_version(struct devlink_info_req *req,
> [DEVLINK_VERSION_RUNNING] = DEVLINK_ATTR_INFO_VERSION_RUNNING,
> };
> struct nlattr *nest;
>- int err;
>+ int len, err;
>+
>+ if (req->compat) {
>+ if (type == DEVLINK_VERSION_RUNNING) {
>+ len = strlcpy(req->buf, version_value, req->len);
If you have multiple running versions, shouldn't you concat them into a
single string for compat?
>+ req->len = max_t(size_t, 0, req->len - len);
>+ }
>+ return 0;
>+ }
>
> if (type >= ARRAY_SIZE(type2attr) || !type2attr[type])
> return -EINVAL;
>@@ -3789,6 +3805,7 @@ devlink_nl_info_fill(struct sk_buff *msg, struct devlink *devlink,
> if (devlink_nl_put_handle(msg, devlink))
> goto err_cancel_msg;
>
>+ memset(&req, 0, sizeof(req));
> req.msg = msg;
> err = devlink->ops->info_get(devlink, &req, extack);
> if (err)
>@@ -5263,6 +5280,39 @@ int devlink_region_snapshot_create(struct devlink_region *region, u64 data_len,
> }
> EXPORT_SYMBOL_GPL(devlink_region_snapshot_create);
>
>+void devlink_compat_running_versions(struct net_device *dev,
Why "versionS?"
>+ char *buf, size_t len)
>+{
>+ struct devlink_port *devlink_port;
>+ struct devlink_info_req req;
>+ struct devlink *devlink;
>+ bool found = false;
>+
>+ mutex_lock(&devlink_mutex);
>+ list_for_each_entry(devlink, &devlink_list, list) {
>+ mutex_lock(&devlink->lock);
>+ list_for_each_entry(devlink_port, &devlink->port_list, list) {
>+ if (devlink_port->type == DEVLINK_PORT_TYPE_ETH ||
>+ devlink_port->type_dev == dev) {
>+ mutex_unlock(&devlink->lock);
>+ found = true;
>+ goto out;
>+ }
>+ }
>+ mutex_unlock(&devlink->lock);
>+ }
>+out:
>+ if (found && devlink->ops->info_get) {
>+ memset(&req, 0, sizeof(req));
>+ req.compat = true;
>+ req.buf = buf;
>+ req.len = len;
>+
>+ devlink->ops->info_get(devlink, &req, NULL);
I don't really like this compat bool and check in "put" functions.
I would rather like to run info_get() in case of both compat and
non-compat in the same way. Then for compat just pickup the info
which is needed and put to buf.
I don't see problem in using netlink skb for that direcly.
>+ }
>+ mutex_unlock(&devlink_mutex);
>+}
>+
> static int __init devlink_module_init(void)
> {
> return genl_register_family(&devlink_nl_family);
>diff --git a/net/core/ethtool.c b/net/core/ethtool.c
>index 158264f7cfaf..176b17d11f08 100644
>--- a/net/core/ethtool.c
>+++ b/net/core/ethtool.c
>@@ -27,6 +27,7 @@
> #include <linux/rtnetlink.h>
> #include <linux/sched/signal.h>
> #include <linux/net.h>
>+#include <net/devlink.h>
> #include <net/xdp_sock.h>
>
> /*
>@@ -803,6 +804,12 @@ static noinline_for_stack int ethtool_get_drvinfo(struct net_device *dev,
> if (ops->get_eeprom_len)
> info.eedump_len = ops->get_eeprom_len(dev);
>
>+ rtnl_unlock();
>+ if (!info.fw_version[0])
>+ devlink_compat_running_versions(dev, info.fw_version,
>+ ARRAY_SIZE(info.fw_version));
ARRAY_SIZE looks odd here. "sizeof()" would be better I think.
>+ rtnl_lock();
>+
> if (copy_to_user(useraddr, &info, sizeof(info)))
> return -EFAULT;
> return 0;
>--
>2.19.2
>
^ permalink raw reply
* [PATCH net-next v2 5/5] net: tls: Add tests for TLS 1.3
From: Dave Watson @ 2019-01-30 21:58 UTC (permalink / raw)
To: netdev@vger.kernel.org, Dave Miller
Cc: Vakul Garg, Boris Pismenny, Aviad Yehezkel, John Fastabend,
Daniel Borkmann
Change most tests to TLS 1.3, while adding tests for previous TLS 1.2
behavior.
Signed-off-by: Dave Watson <davejwatson@fb.com>
---
tools/testing/selftests/net/tls.c | 76 ++++++++++++++++++++++++++++++-
1 file changed, 75 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/net/tls.c b/tools/testing/selftests/net/tls.c
index c356f481de79..4ac50ccb3272 100644
--- a/tools/testing/selftests/net/tls.c
+++ b/tools/testing/selftests/net/tls.c
@@ -42,7 +42,7 @@ FIXTURE_SETUP(tls)
len = sizeof(addr);
memset(&tls12, 0, sizeof(tls12));
- tls12.info.version = TLS_1_2_VERSION;
+ tls12.info.version = TLS_1_3_VERSION;
tls12.info.cipher_type = TLS_CIPHER_AES_GCM_128;
addr.sin_family = AF_INET;
@@ -825,4 +825,78 @@ TEST(keysizes) {
close(cfd);
}
+TEST(tls12) {
+ int fd, cfd;
+ bool notls;
+
+ struct tls12_crypto_info_aes_gcm_128 tls12;
+ struct sockaddr_in addr;
+ socklen_t len;
+ int sfd, ret;
+
+ notls = false;
+ len = sizeof(addr);
+
+ memset(&tls12, 0, sizeof(tls12));
+ tls12.info.version = TLS_1_2_VERSION;
+ tls12.info.cipher_type = TLS_CIPHER_AES_GCM_128;
+
+ addr.sin_family = AF_INET;
+ addr.sin_addr.s_addr = htonl(INADDR_ANY);
+ addr.sin_port = 0;
+
+ fd = socket(AF_INET, SOCK_STREAM, 0);
+ sfd = socket(AF_INET, SOCK_STREAM, 0);
+
+ ret = bind(sfd, &addr, sizeof(addr));
+ ASSERT_EQ(ret, 0);
+ ret = listen(sfd, 10);
+ ASSERT_EQ(ret, 0);
+
+ ret = getsockname(sfd, &addr, &len);
+ ASSERT_EQ(ret, 0);
+
+ ret = connect(fd, &addr, sizeof(addr));
+ ASSERT_EQ(ret, 0);
+
+ ret = setsockopt(fd, IPPROTO_TCP, TCP_ULP, "tls", sizeof("tls"));
+ if (ret != 0) {
+ notls = true;
+ printf("Failure setting TCP_ULP, testing without tls\n");
+ }
+
+ if (!notls) {
+ ret = setsockopt(fd, SOL_TLS, TLS_TX, &tls12,
+ sizeof(tls12));
+ ASSERT_EQ(ret, 0);
+ }
+
+ cfd = accept(sfd, &addr, &len);
+ ASSERT_GE(cfd, 0);
+
+ if (!notls) {
+ ret = setsockopt(cfd, IPPROTO_TCP, TCP_ULP, "tls",
+ sizeof("tls"));
+ ASSERT_EQ(ret, 0);
+
+ ret = setsockopt(cfd, SOL_TLS, TLS_RX, &tls12,
+ sizeof(tls12));
+ ASSERT_EQ(ret, 0);
+ }
+
+ close(sfd);
+
+ char const *test_str = "test_read";
+ int send_len = 10;
+ char buf[10];
+
+ send_len = strlen(test_str) + 1;
+ EXPECT_EQ(send(fd, test_str, send_len, 0), send_len);
+ EXPECT_NE(recv(cfd, buf, send_len, 0), -1);
+ EXPECT_EQ(memcmp(buf, test_str, send_len), 0);
+
+ close(fd);
+ close(cfd);
+}
+
TEST_HARNESS_MAIN
--
2.17.1
^ permalink raw reply related
* Re: net: phylink: dsa: mv88e6xxx: flaky link detection on switch ports with internal PHYs
From: Andrew Lunn @ 2019-01-30 22:38 UTC (permalink / raw)
To: John David Anglin; +Cc: Russell King, Vivien Didelot, Florian Fainelli, netdev
In-Reply-To: <2ea9fd81-f92d-9505-dd0b-bdd0f67d8ce7@bell.net>
On Wed, Jan 30, 2019 at 05:24:53PM -0500, John David Anglin wrote:
> On 2019-01-30 12:28 p.m., Andrew Lunn wrote:
> > You need active low interrupts. Without it, i think you are always
> > going to have race conditions which will cause interrupts to get
> > stuck/lost.
> I don't know if this is a hardware limitation or not, but currently the
> armada 37xx doesn't support
> level interrupts:
Yes, i had a quick look at the pinctrl driver. It only has code for
edges.
I'd suggest you take a look at the datasheet for the 37xx and check
what the hardware actually supports. You might need to extend the
driver.
Andrew
^ permalink raw reply
* Re: [PATCH net-next v2 10/12] net: dsa: Wire up multicast IGMP snooping attribute notification
From: Andrew Lunn @ 2019-01-30 22:46 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev, vivien.didelot, davem, idosch, jiri, ilias.apalodimas,
ivan.khoronzhuk, roopa, nikolay
In-Reply-To: <20190130005548.2212-11-f.fainelli@gmail.com>
> @@ -425,6 +425,8 @@ struct dsa_switch_ops {
> /*
> * Multicast database
> */
> + int (*port_multicast_toggle)(struct dsa_switch *ds, int port,
> + bool mc_disabled);
> int (*port_mdb_prepare)(struct dsa_switch *ds, int port,
> const struct switchdev_obj_port_mdb *mdb);
> void (*port_mdb_add)(struct dsa_switch *ds, int port,
Hi Florian
I took a second look at this, after you asked the question.
What i see above is that port_multicast_toggle is different to
port_mdb_prepare and port_mdb_add. From this context, it looks like
there should not be a tab between int and (*port_multicast_toggle).
However, when you look at the real file, it becomes clear that it is
actually normal to have a tab between the type and the name of the
function pointer, and that port_mdb_prepare and port_mdb_add are
different to the rest.
So you patch is O.K.
Andrew
^ permalink raw reply
* Re: [PATCH v2 net 0/7] virtio_net: Fix problems around XDP tx and napi_tx
From: David Miller @ 2019-01-30 22:03 UTC (permalink / raw)
To: makita.toshiaki; +Cc: mst, jasowang, netdev, virtualization, willemb, brouer
In-Reply-To: <1548722759-2470-1-git-send-email-makita.toshiaki@lab.ntt.co.jp>
From: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Date: Tue, 29 Jan 2019 09:45:52 +0900
> While I'm looking into how to account standard tx counters on XDP tx
> processing, I found several bugs around XDP tx and napi_tx.
>
> Patch1: Fix oops on error path. Patch2 depends on this.
> Patch2: Fix memory corruption on freeing xdp_frames with napi_tx enabled.
> Patch3: Minor fix patch5 depends on.
> Patch4: Fix memory corruption on processing xdp_frames when XDP is disabled.
> Also patch5 depends on this.
> Patch5: Fix memory corruption on processing xdp_frames while XDP is being
> disabled.
> Patch6: Minor fix patch7 depends on.
> Patch7: Fix memory corruption on freeing sk_buff or xdp_frames when a normal
> queue is reused for XDP and vise versa.
>
> v2:
> - patch5: Make rcu_assign_pointer/synchronize_net conditional instead of
> _virtnet_set_queues.
> - patch7: Use napi_consume_skb() instead of dev_consume_skb_any()
>
> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Series applied.
^ permalink raw reply
* Re: [PATCH net 0/2] erspan: always reports output key to userspace
From: David Miller @ 2019-01-30 22:00 UTC (permalink / raw)
To: lorenzo.bianconi; +Cc: netdev, u9012063
In-Reply-To: <cover.1548709944.git.lorenzo.bianconi@redhat.com>
From: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
Date: Mon, 28 Jan 2019 22:23:47 +0100
> Erspan protocol relies on output key to set session id header field.
> However TUNNEL_KEY bit is cleared in order to not add key field to
> the external GRE header and so the configured o_key is not reported
> to userspace.
> Fix the issue adding TUNNEL_KEY bit to the o_flags parameter dumping
> device info
Series applied, thanks!
^ permalink raw reply
* Re: [PATCH] selftests: net: fix "from" match test in fib_rule_tests.sh
From: David Ahern @ 2019-01-30 23:19 UTC (permalink / raw)
To: Marcelo Henrique Cerri, David S. Miller, Shuah Khan, netdev,
linux-kselftest, linux-kernel
In-Reply-To: <20190130181903.16465-1-marcelo.cerri@canonical.com>
On 1/30/19 11:19 AM, Marcelo Henrique Cerri wrote:
> Fix the IPv4 address of the dummy0 interface and ensure that ip_forward
> is enabled in the network space to get a valid response when checking
> for routes between the gateway and other hosts.
you need a Fixes tag and to cc the author of said commit.
>
> Signed-off-by: Marcelo Henrique Cerri <marcelo.cerri@canonical.com>
> ---
> tools/testing/selftests/net/fib_rule_tests.sh | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/net/fib_rule_tests.sh b/tools/testing/selftests/net/fib_rule_tests.sh
> index d4cfb6a7a086..552a9784e759 100755
> --- a/tools/testing/selftests/net/fib_rule_tests.sh
> +++ b/tools/testing/selftests/net/fib_rule_tests.sh
> @@ -54,9 +54,11 @@ setup()
>
> $IP link add dummy0 type dummy
> $IP link set dev dummy0 up
> - $IP address add 198.51.100.1/24 dev dummy0
> + $IP address add 192.51.100.1/24 dev dummy0
> $IP -6 address add 2001:db8:1::1/64 dev dummy0
>
> + ip netns exec testns sysctl -w net.ipv4.ip_forward=1
ip_forward is not relevant for this test. It just adds routes and rules
and uses ip route get to verify; no traffic is actually genrated.
^ permalink raw reply
* Re: BUG: KASAN: double-free or invalid-free in ip_defrag after upgrade from 4.19.13
From: Eric Dumazet @ 2019-01-30 23:16 UTC (permalink / raw)
To: Ivan Babrou, Greg Kroah-Hartman
Cc: Michal Kubecek, Linux Kernel Network Developers, David S. Miller,
Ignat Korchagin, Shawn Bohrer, Jakub Sitnicki
In-Reply-To: <CANn89iJ_LOc+yPtyQrRQieniy5Pga=CVKorO4qdvHXCqKgyaHw@mail.gmail.com>
On Wed, Jan 30, 2019 at 3:13 PM Eric Dumazet <edumazet@google.com> wrote:
>
> On Wed, Jan 30, 2019 at 3:09 PM Ivan Babrou <ivan@cloudflare.com> wrote:
> >
> > Eric,
> >
> > Are you going to propose the change then?
> >
> > I'm happy to test it out.
> >
>
> This is indeed a bug in linux stable tree only.
>
> The err=-EINVAL move was part of a patch that was not backported
> (since it was not a bug fix)
>
> commit 0ff89efb524631ac9901b81446b453c29711c376
> Author: Peter Oskolkov <posk@google.com>
> Date: Tue Aug 28 11:36:19 2018 -0700
>
> ip: fail fast on IP defrag errors
>
>
Greg, the fix for 4.19 (and maybe other stable trees ?) would be :
diff --git a/net/ipv4/ip_fragment.c b/net/ipv4/ip_fragment.c
index f8bbd693c19c247e41839c2d0b5318ca51b23ee8..d95b32af4a0e3f552405c9e61cc372729834160c
100644
--- a/net/ipv4/ip_fragment.c
+++ b/net/ipv4/ip_fragment.c
@@ -425,6 +425,7 @@ static int ip_frag_queue(struct ipq *qp, struct
sk_buff *skb)
* fragment.
*/
+ err = -EINVAL;
/* Find out where to put this fragment. */
prev_tail = qp->q.fragments_tail;
if (!prev_tail)
@@ -501,7 +502,6 @@ static int ip_frag_queue(struct ipq *qp, struct
sk_buff *skb)
discard_qp:
inet_frag_kill(&qp->q);
- err = -EINVAL;
__IP_INC_STATS(net, IPSTATS_MIB_REASM_OVERLAPS);
err:
kfree_skb(skb);
> > On Wed, Jan 30, 2019 at 3:00 PM Michal Kubecek <mkubecek@suse.cz> wrote:
> > >
> > > On Wed, Jan 30, 2019 at 02:26:32PM -0800, Ivan Babrou wrote:
> > > > Hey,
> > > >
> > > > Continuing from this thread earlier today:
> > > >
> > > > * https://marc.info/?t=154886729100001&r=1&w=2
> > > >
> > > > We fired up KASAN enabled kernel one one of those machine and this is
> > > > what we saw:
> > > ...
> > > > This commit from 4.19.14 seems relevant:
> > > >
> > > > * https://github.com/torvalds/linux/commit/d5f9565c8d5ad3cf94982223cfcef1169b0bb60f
> > > >
> > > > As a reminder, we upgraded from 4.19.13 and started seeing crashes.
> > >
> > > Unfortunately I'm on vacation this week so that my capability to look
> > > deeper into this is limited but there seems to be one obvious problem
> > > with the 4.19.y backport: in mainline, there is
> > >
> > > err = -EINVAL;
> > >
> > > right on top of the "Find out where to put this fragment." comment which
> > > had been added by commit 0ff89efb5246 ("ip: fail fast on IP defrag
> > > errors"). In 4.19.y backport of the commit, this assignment is missing
> > > so that the value of err at this point comes from earlier
> > > pskb_trim_rcsum() call so that it must be zero and if we take any of the
> > > "goto err" added by commit d5f9565c8d5a, we drop the packet by calling
> > > kfree_skb() but return zero so that caller doesn't know about it.
> > >
> > > Michal Kubecek
> > >
^ permalink raw reply
* Re: [PATCH] : net : hso : do not call unregister_netdev if not registered
From: David Miller @ 2019-01-30 21:58 UTC (permalink / raw)
To: tuba; +Cc: netdev
In-Reply-To: <1548692918696.27977@ece.ufl.edu>
From: "Yavuz, Tuba" <tuba@ece.ufl.edu>
Date: Mon, 28 Jan 2019 16:28:38 +0000
> On an error path inside the hso_create_net_device function of the hso
> driver, hso_free_net_device gets called. This causes potentially a
> negative reference count in the net device if register_netdev has not
> been called yet as hso_free_net_device calls unregister_netdev
> regardless. I think the driver should distinguish these cases and call
> unregister_netdev only if register_netdev has been called.
>
> Reported-by: Tuba Yavuz <tuba@ece.ufl.edu>
> Signed-off-by: Tuba Yavuz <tuba@ece.ufl.edu>
This does not apply cleanly to the net tree.
Also, please stop putting those spaces after the subsystem prefixes
in your Subject line. Put the colon character immediately after
each subsystem prefix.
Thank you.
^ permalink raw reply
* Re: [PATCH net-next v2 6/7] nfp: devlink: report the running and flashed versions
From: Jiri Pirko @ 2019-01-30 21:57 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, oss-drivers, andrew, f.fainelli, mkubecek, eugenem,
jonathan.lemon
In-Reply-To: <20190130190513.25718-7-jakub.kicinski@netronome.com>
Wed, Jan 30, 2019 at 08:05:12PM CET, jakub.kicinski@netronome.com wrote:
>Report versions of firmware components using the new NSP command.
>
>Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
>---
> .../net/ethernet/netronome/nfp/nfp_devlink.c | 86 +++++++++++++++++++
> include/net/devlink.h | 11 +++
> 2 files changed, 97 insertions(+)
>
>diff --git a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
>index 9857fa663adf..fade37d2b796 100644
>--- a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
>+++ b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
>@@ -207,11 +207,59 @@ nfp_devlink_versions_get_hwinfo(struct nfp_pf *pf, struct devlink_info_req *req)
> return 0;
> }
>
>+static const struct nfp_devlink_versions {
>+ enum nfp_nsp_versions id;
>+ const char *key;
>+} nfp_devlink_versions_nsp[] = {
>+ { NFP_VERSIONS_BUNDLE, "fw.bundle_id", },
>+ { NFP_VERSIONS_BSP, DEVLINK_VERSION_GENERIC_FW_MGMT, },
>+ { NFP_VERSIONS_CPLD, "fw.cpld", },
>+ { NFP_VERSIONS_APP, DEVLINK_VERSION_GENERIC_FW_APP, },
>+ { NFP_VERSIONS_UNDI, DEVLINK_VERSION_GENERIC_FW_UNDI, },
>+ { NFP_VERSIONS_NCSI, DEVLINK_VERSION_GENERIC_FW_NCSI, },
>+ { NFP_VERSIONS_CFGR, "chip.init", },
>+};
>+
>+static int
>+nfp_devlink_versions_get_nsp(struct devlink_info_req *req, bool flash,
>+ const u8 *buf, unsigned int size)
>+{
>+ enum devlink_version_type type;
>+ unsigned int i;
>+ int err;
>+
>+ type = flash ? DEVLINK_VERSION_STORED : DEVLINK_VERSION_RUNNING;
>+
>+ for (i = 0; i < ARRAY_SIZE(nfp_devlink_versions_nsp); i++) {
>+ const struct nfp_devlink_versions *info;
>+ const char *version;
>+
>+ info = &nfp_devlink_versions_nsp[i];
>+
>+ version = nfp_nsp_versions_get(info->id, flash, buf, size);
>+ if (IS_ERR(version)) {
>+ if (PTR_ERR(version) == -ENOENT)
>+ continue;
>+ else
>+ return PTR_ERR(version);
>+ }
>+
>+ err = devlink_info_report_version(req, type,
>+ info->key, version);
>+ if (err)
>+ return err;
>+ }
>+
>+ return 0;
>+}
>+
> static int
> nfp_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
> struct netlink_ext_ack *extack)
> {
> struct nfp_pf *pf = devlink_priv(devlink);
>+ struct nfp_nsp *nsp;
>+ char *buf = NULL;
> const char *sn;
> int err;
>
>@@ -226,7 +274,45 @@ nfp_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
> return err;
> }
>
>+ nsp = nfp_nsp_open(pf->cpp);
>+ if (IS_ERR(nsp)) {
>+ NL_SET_ERR_MSG_MOD(extack, "can't access NSP");
>+ return PTR_ERR(nsp);
>+ }
>+
>+ if (nfp_nsp_has_versions(nsp)) {
>+ buf = kzalloc(NFP_NSP_VERSION_BUFSZ, GFP_KERNEL);
>+ if (!buf) {
>+ err = -ENOMEM;
>+ goto err_close_nsp;
>+ }
>+
>+ err = nfp_nsp_versions(nsp, buf, NFP_NSP_VERSION_BUFSZ);
>+ if (err)
>+ goto err_free_buf;
>+
>+ err = nfp_devlink_versions_get_nsp(req, false,
>+ buf, NFP_NSP_VERSION_BUFSZ);
>+ if (err)
>+ goto err_free_buf;
>+
>+ err = nfp_devlink_versions_get_nsp(req, true,
>+ buf, NFP_NSP_VERSION_BUFSZ);
>+ if (err)
>+ goto err_free_buf;
>+
>+ kfree(buf);
>+ }
>+
>+ nfp_nsp_close(nsp);
>+
> return nfp_devlink_versions_get_hwinfo(pf, req);
>+
>+err_free_buf:
>+ kfree(buf);
>+err_close_nsp:
>+ nfp_nsp_close(nsp);
>+ return err;
> }
>
> const struct devlink_ops nfp_devlink_ops = {
>diff --git a/include/net/devlink.h b/include/net/devlink.h
>index 3d553cc6693d..c678ed0cb099 100644
>--- a/include/net/devlink.h
>+++ b/include/net/devlink.h
>@@ -433,6 +433,17 @@ enum devlink_param_wol_types {
> /* Revision of board design */
> #define DEVLINK_VERSION_GENERIC_BOARD_REV "board.rev"
>
>+/* Control processor FW version, FW is responsible for house keeping tasks,
>+ * PHY control etc.
>+ */
>+#define DEVLINK_VERSION_GENERIC_FW_MGMT "fw.mgmt"
>+/* Data path microcode controlling high-speed packet processing */
>+#define DEVLINK_VERSION_GENERIC_FW_APP "fw.app"
>+/* UNDI software version */
>+#define DEVLINK_VERSION_GENERIC_FW_UNDI "fw.undi"
>+/* NCSI support/handler version */
>+#define DEVLINK_VERSION_GENERIC_FW_NCSI "fw.ncsi"
Same here. Also, please put "INFO" in the names to respect the namespacing
>+
> enum devlink_version_type {
> DEVLINK_VERSION_FIXED,
> DEVLINK_VERSION_STORED,
>--
>2.19.2
>
^ permalink raw reply
* Re: BUG: KASAN: double-free or invalid-free in ip_defrag after upgrade from 4.19.13
From: Eric Dumazet @ 2019-01-30 23:13 UTC (permalink / raw)
To: Ivan Babrou, Greg Kroah-Hartman
Cc: Michal Kubecek, Linux Kernel Network Developers, David S. Miller,
Ignat Korchagin, Shawn Bohrer, Jakub Sitnicki
In-Reply-To: <CABWYdi09eRHCscfFBJ=9-ZPtP57eRO_grFmrDVvFXGcr8JmDtA@mail.gmail.com>
On Wed, Jan 30, 2019 at 3:09 PM Ivan Babrou <ivan@cloudflare.com> wrote:
>
> Eric,
>
> Are you going to propose the change then?
>
> I'm happy to test it out.
>
This is indeed a bug in linux stable tree only.
The err=-EINVAL move was part of a patch that was not backported
(since it was not a bug fix)
commit 0ff89efb524631ac9901b81446b453c29711c376
Author: Peter Oskolkov <posk@google.com>
Date: Tue Aug 28 11:36:19 2018 -0700
ip: fail fast on IP defrag errors
> On Wed, Jan 30, 2019 at 3:00 PM Michal Kubecek <mkubecek@suse.cz> wrote:
> >
> > On Wed, Jan 30, 2019 at 02:26:32PM -0800, Ivan Babrou wrote:
> > > Hey,
> > >
> > > Continuing from this thread earlier today:
> > >
> > > * https://marc.info/?t=154886729100001&r=1&w=2
> > >
> > > We fired up KASAN enabled kernel one one of those machine and this is
> > > what we saw:
> > ...
> > > This commit from 4.19.14 seems relevant:
> > >
> > > * https://github.com/torvalds/linux/commit/d5f9565c8d5ad3cf94982223cfcef1169b0bb60f
> > >
> > > As a reminder, we upgraded from 4.19.13 and started seeing crashes.
> >
> > Unfortunately I'm on vacation this week so that my capability to look
> > deeper into this is limited but there seems to be one obvious problem
> > with the 4.19.y backport: in mainline, there is
> >
> > err = -EINVAL;
> >
> > right on top of the "Find out where to put this fragment." comment which
> > had been added by commit 0ff89efb5246 ("ip: fail fast on IP defrag
> > errors"). In 4.19.y backport of the commit, this assignment is missing
> > so that the value of err at this point comes from earlier
> > pskb_trim_rcsum() call so that it must be zero and if we take any of the
> > "goto err" added by commit d5f9565c8d5a, we drop the packet by calling
> > kfree_skb() but return zero so that caller doesn't know about it.
> >
> > Michal Kubecek
> >
^ permalink raw reply
* Re: [RFC 00/14] netlink/hierarchical stats
From: Roopa Prabhu @ 2019-01-30 22:14 UTC (permalink / raw)
To: Jakub Kicinski
Cc: David Miller, oss-drivers, netdev, Jiří Pírko,
Florian Fainelli, Andrew Lunn, Michal Kubecek, David Ahern,
Simon Horman, Brandeburg, Jesse, maciejromanfijalkowski,
vasundhara-v.volam, Michael Chan, shalomt, Ido Schimmel
In-Reply-To: <20190128234507.32028-1-jakub.kicinski@netronome.com>
On Mon, Jan 28, 2019 at 3:45 PM Jakub Kicinski
<jakub.kicinski@netronome.com> wrote:
>
> Hi!
>
> As I tried to explain in my slides at netconf 2018 we are lacking
> an expressive, standard API to report device statistics.
>
> Networking silicon generally maintains some IEEE 802.3 and/or RMON
> statistics. Today those all end up in ethtool -S. Here is a simple
> attempt (admittedly very imprecise) of counting how many names driver
> authors invented for IETF RFC2819 etherStatsPkts512to1023Octets
> statistics (RX and TX):
>
> $ git grep '".*512.*1023.*"' -- drivers/net/ | \
> sed -e 's/.*"\(.*\)".*/\1/' | sort | uniq | wc -l
> 63
>
> Interestingly only two drivers in the tree use the name the standard
> gave us (etherStatsPkts512to1023, modulo case).
>
> I set out to working on this set in an attempt to give drivers a way
> to express clearly to user space standard-compliant counters.
>
> Second most common use for custom statistics is per-queue counters.
> This is where the "hierarchical" part of this set comes in, as
> groups can be nested, and user space tools can handle the aggregation
> inside the groups if needed.
>
> This set also tries to address the problem of users not knowing if
> a statistic is reported by hardware or the driver. Many modern drivers
> use some prefix in ethtool -S to indicate MAC/PHY stats. At a quick
> glance: Netronome uses "mac.", Intel "port." and Mellanox "_phy".
> In this set, netlink attributes describe whether a group of statistics
> is RX or TX, maintained by device or driver.
>
> The purpose of this patch set is _not_ to replace ethtool -S. It is
> an incredibly useful tool, and we will certainly continue using it.
> However, for standard-based and commonly maintained statistics a more
> structured API seems warranted.
>
> There are two things missing from these patches, which I initially
> planned to address as well: filtering, and refresh rate control.
>
> Filtering doesn't need much explanation, users should be able to request
> only a subset of statistics (like only SW stats or only given ID). The
> bitmap of statistics in each group is there for filtering later on.
>
> By refresh control I mean the ability for user space to indicate how
> "fresh" values it expects. Sometimes reading the HW counters requires
> slow register reads or FW communication, in such cases drivers may cache
> the result. (Privileged) user space should be able to add a "not older
> than" timestamp to indicate how fresh statistics it expects. And vice
> versa, drivers can then also put the timestamp of when the statistics
> were last refreshed in the dump for more precise bandwidth estimation.
Jakub, Glad to see hw stats in the RTM_*STATS api. I do see you
mention 'partial' support for ethtool stats. I understand the reason
you say its partial.
But while at it, why not also include the ability to have driver
extensible stats here ? ie make it complete. We have talked about
making all hw stats available
via the RTM_*STATS api in the past..., so just want to make sure the
new HSTATS infra you are adding to the RTM_*STATS api
covers or at-least makes it possible to include driver extensible
stats in the future where the driver gets to define the stats id +
value (This is very useful).
It would be nice if you can account for that in this new HSTATS API.
>
> Jakub Kicinski (14):
> nfp: remove unused structure
> nfp: constify parameter to nfp_port_from_netdev()
> net: hstats: add basic/core functionality
> net: hstats: allow hierarchies to be built
> nfp: very basic hstat support
> net: hstats: allow iterators
> net: hstats: help in iteration over directions
> nfp: hstats: make use of iteration for direction
> nfp: hstats: add driver and device per queue statistics
> net: hstats: add IEEE 802.3 and common IETF MIB/RMON stats
> nfp: hstats: add IEEE/RMON ethernet port/MAC stats
> net: hstats: add markers for partial groups
> nfp: hstats: add a partial group of per-8021Q prio stats
> Documentation: networking: describe new hstat API
>
> Documentation/networking/hstats.rst | 590 +++++++++++++++
> .../networking/hstats_flow_example.dot | 11 +
> Documentation/networking/index.rst | 1 +
> drivers/net/ethernet/netronome/nfp/Makefile | 1 +
> .../net/ethernet/netronome/nfp/nfp_hstat.c | 474 ++++++++++++
> drivers/net/ethernet/netronome/nfp/nfp_main.c | 1 +
> drivers/net/ethernet/netronome/nfp/nfp_main.h | 2 +
> drivers/net/ethernet/netronome/nfp/nfp_net.h | 10 +-
> .../ethernet/netronome/nfp/nfp_net_common.c | 1 +
> .../net/ethernet/netronome/nfp/nfp_net_repr.h | 2 +-
> drivers/net/ethernet/netronome/nfp/nfp_port.c | 2 +-
> drivers/net/ethernet/netronome/nfp/nfp_port.h | 2 +-
> include/linux/netdevice.h | 9 +
> include/net/hstats.h | 176 +++++
> include/uapi/linux/if_link.h | 107 +++
> net/core/Makefile | 2 +-
> net/core/hstats.c | 682 ++++++++++++++++++
> net/core/rtnetlink.c | 21 +
> 18 files changed, 2084 insertions(+), 10 deletions(-)
> create mode 100644 Documentation/networking/hstats.rst
> create mode 100644 Documentation/networking/hstats_flow_example.dot
> create mode 100644 drivers/net/ethernet/netronome/nfp/nfp_hstat.c
> create mode 100644 include/net/hstats.h
> create mode 100644 net/core/hstats.c
>
> --
> 2.19.2
>
^ permalink raw reply
* Re: [PATCH] net: esp4: Fix double free on esp4 functions
From: David Miller @ 2019-01-30 21:53 UTC (permalink / raw)
To: ramin.blackhat; +Cc: herbert, steffen.klassert, netdev
In-Reply-To: <20190130213542.17313-1-ramin.blackhat@gmail.com>
From: Ramin Farajpour Cami <ramin.blackhat@gmail.com>
Date: Wed, 30 Jan 2019 21:35:42 +0000
> key/tmp is being kfree'd twice,once in the "aalg_desc->uinfo.auth.icv_fullbits / 8 != crypto_aead_authsize(aead)" call
> to "free_key",twice When "crypto_aead_setauthsize(aead, x->aalg->alg_trunc_len / 8)" fails call to again "free_key",
>
> Signed-off-by: Ramin Farajpour Cami <ramin.blackhat@gmail.com>
> ---
> net/ipv4/esp4.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
> index 5459f41fc26f..5a66e47641b0 100644
> --- a/net/ipv4/esp4.c
> +++ b/net/ipv4/esp4.c
> @@ -467,6 +467,7 @@ int esp_output_tail(struct xfrm_state *x, struct sk_buff *skb, struct esp_info *
>
> error_free:
> kfree(tmp);
> + tmp = NULL;
> error:
> return err;
> }
This makes no sense at all, the function returns right after the kfree() and
tmp is never referenced again!
^ permalink raw reply
* Re: [PATCHv4 0/3]: net: dsa: mt7530: support MT7530 in the MT7621 SoC
From: David Miller @ 2019-01-30 22:30 UTC (permalink / raw)
To: gerg
Cc: sean.wang, linux-mediatek, bjorn, andrew, vivien.didelot,
f.fainelli, netdev, rene, john, neil
In-Reply-To: <20190130012406.28271-1-gerg@kernel.org>
From: gerg@kernel.org
Date: Wed, 30 Jan 2019 11:24:03 +1000
> This is the fourth version of a patch series supporting the MT7530 switch
> as used in the MediaTek MT7621 SoC. Unlike the MediaTek MT7623 the MT7621
> is built around a dual core MIPS CPU architecture. But inside it uses
> basically the same 7530 switch.
...
Series applied, and yes a COMPILE_TEST Kconfig dep would be great.
^ permalink raw reply
* Re: [PATCH net] vhost: fix OOB in get_rx_bufs()
From: David Miller @ 2019-01-30 22:31 UTC (permalink / raw)
To: mst; +Cc: jasowang, stefanha, kvm, virtualization, netdev, linux-kernel
In-Reply-To: <20190129203157-mutt-send-email-mst@kernel.org>
From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Tue, 29 Jan 2019 20:36:31 -0500
> If it helps I can include most virtio stuff in my pull requests instead.
> Or if that can't work since there's too often a dependency on net-next,
> maybe Jason wants to create a tree and send pull requests to you. Let
> us know if that will help, and which of the options looks better from
> your POV.
Thanks for offering Michael, I really appreciate it.
Let me think about the logistics of that and how it may or may not
help me with my backlog.
^ permalink raw reply
* Re: BUG: KASAN: double-free or invalid-free in ip_defrag after upgrade from 4.19.13
From: Ivan Babrou @ 2019-01-30 23:09 UTC (permalink / raw)
To: Michal Kubecek
Cc: Linux Kernel Network Developers, David S. Miller, Eric Dumazet,
Ignat Korchagin, Shawn Bohrer, Jakub Sitnicki
In-Reply-To: <20190130230001.GL24651@unicorn.suse.cz>
Eric,
Are you going to propose the change then?
I'm happy to test it out.
On Wed, Jan 30, 2019 at 3:00 PM Michal Kubecek <mkubecek@suse.cz> wrote:
>
> On Wed, Jan 30, 2019 at 02:26:32PM -0800, Ivan Babrou wrote:
> > Hey,
> >
> > Continuing from this thread earlier today:
> >
> > * https://marc.info/?t=154886729100001&r=1&w=2
> >
> > We fired up KASAN enabled kernel one one of those machine and this is
> > what we saw:
> ...
> > This commit from 4.19.14 seems relevant:
> >
> > * https://github.com/torvalds/linux/commit/d5f9565c8d5ad3cf94982223cfcef1169b0bb60f
> >
> > As a reminder, we upgraded from 4.19.13 and started seeing crashes.
>
> Unfortunately I'm on vacation this week so that my capability to look
> deeper into this is limited but there seems to be one obvious problem
> with the 4.19.y backport: in mainline, there is
>
> err = -EINVAL;
>
> right on top of the "Find out where to put this fragment." comment which
> had been added by commit 0ff89efb5246 ("ip: fail fast on IP defrag
> errors"). In 4.19.y backport of the commit, this assignment is missing
> so that the value of err at this point comes from earlier
> pskb_trim_rcsum() call so that it must be zero and if we take any of the
> "goto err" added by commit d5f9565c8d5a, we drop the packet by calling
> kfree_skb() but return zero so that caller doesn't know about it.
>
> Michal Kubecek
>
^ permalink raw reply
* [PATCH net-next] net: tls: Set async_capable for tls zerocopy only if we see EINPROGRESS
From: Dave Watson @ 2019-01-30 22:08 UTC (permalink / raw)
To: netdev@vger.kernel.org, Dave Miller
Cc: Vakul Garg, Boris Pismenny, Aviad Yehezkel, John Fastabend,
Daniel Borkmann
Currently we don't zerocopy if the crypto framework async bit is set.
However some crypto algorithms (such as x86 AESNI) support async,
but in the context of sendmsg, will never run asynchronously. Instead,
check for actual EINPROGRESS return code before assuming algorithm is
async.
Signed-off-by: Dave Watson <davejwatson@fb.com>
---
include/net/tls.h | 1 +
net/tls/tls_sw.c | 4 ++--
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/include/net/tls.h b/include/net/tls.h
index 4592606e136a..eb73e62ac8c9 100644
--- a/include/net/tls.h
+++ b/include/net/tls.h
@@ -139,6 +139,7 @@ struct tls_sw_context_tx {
struct list_head tx_list;
atomic_t encrypt_pending;
int async_notify;
+ int async_capable;
#define BIT_TX_SCHEDULED 0
unsigned long tx_bitmask;
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 3f2a6af27e62..2864ad16c2d9 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -673,6 +673,7 @@ static int tls_push_record(struct sock *sk, int flags,
tls_merge_open_record(sk, rec, tmp, orig_end);
}
}
+ ctx->async_capable = 1;
return rc;
} else if (split) {
msg_pl = &tmp->msg_plaintext;
@@ -814,8 +815,7 @@ int tls_sw_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
long timeo = sock_sndtimeo(sk, msg->msg_flags & MSG_DONTWAIT);
struct tls_context *tls_ctx = tls_get_ctx(sk);
struct tls_sw_context_tx *ctx = tls_sw_ctx_tx(tls_ctx);
- struct crypto_tfm *tfm = crypto_aead_tfm(ctx->aead_send);
- bool async_capable = tfm->__crt_alg->cra_flags & CRYPTO_ALG_ASYNC;
+ bool async_capable = ctx->async_capable;
unsigned char record_type = TLS_RECORD_TYPE_DATA;
bool is_kvec = iov_iter_is_kvec(&msg->msg_iter);
bool eor = !(msg->msg_flags & MSG_MORE);
--
2.17.1
^ permalink raw reply related
* Re: [PATCH] net: esp4: Fix double free on esp4 functions
From: Eric Dumazet @ 2019-01-30 23:07 UTC (permalink / raw)
To: Ramin Farajpour Cami, davem; +Cc: herbert, steffen.klassert, netdev
In-Reply-To: <20190130213542.17313-1-ramin.blackhat@gmail.com>
On 01/30/2019 01:35 PM, Ramin Farajpour Cami wrote:
> key/tmp is being kfree'd twice,once in the "aalg_desc->uinfo.auth.icv_fullbits / 8 != crypto_aead_authsize(aead)" call
> to "free_key",twice When "crypto_aead_setauthsize(aead, x->aalg->alg_trunc_len / 8)" fails call to again "free_key",
>
> Signed-off-by: Ramin Farajpour Cami <ramin.blackhat@gmail.com>
> ---
> net/ipv4/esp4.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
> index 5459f41fc26f..5a66e47641b0 100644
> --- a/net/ipv4/esp4.c
> +++ b/net/ipv4/esp4.c
> @@ -467,6 +467,7 @@ int esp_output_tail(struct xfrm_state *x, struct sk_buff *skb, struct esp_info *
>
> error_free:
> kfree(tmp);
> + tmp = NULL;
Clearing tmp right before a "return err;" has no effect at all.
> error:
> return err;
> }
> @@ -959,7 +960,7 @@ static int esp_init_authenc(struct xfrm_state *x)
>
> free_key:
> kfree(key);
> -
> + key = NULL;
Same here, this is essentially dead code.
> error:
> return err;
> }
>
^ permalink raw reply
* Re: [PATCH net-next v2 4/7] nfp: devlink: report fixed versions
From: Jiri Pirko @ 2019-01-30 21:56 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, oss-drivers, andrew, f.fainelli, mkubecek, eugenem,
jonathan.lemon
In-Reply-To: <20190130190513.25718-5-jakub.kicinski@netronome.com>
Wed, Jan 30, 2019 at 08:05:10PM CET, jakub.kicinski@netronome.com wrote:
>Report information about the hardware.
>
>RFCv2:
> - add defines for board IDs which are likely to be reusable for
> other drivers (Jiri).
>
>Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
>---
> .../net/ethernet/netronome/nfp/nfp_devlink.c | 37 ++++++++++++++++++-
> include/net/devlink.h | 5 +++
> 2 files changed, 41 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
>index cb3ef7e46614..9857fa663adf 100644
>--- a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
>+++ b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
>@@ -172,6 +172,41 @@ static int nfp_devlink_eswitch_mode_set(struct devlink *devlink, u16 mode,
> return ret;
> }
>
>+static const struct nfp_devlink_versions_simple {
>+ const char *key;
>+ const char *hwinfo;
>+} nfp_devlink_versions_hwinfo[] = {
>+ { DEVLINK_VERSION_GENERIC_BOARD_ID, "assembly.partno", },
>+ { DEVLINK_VERSION_GENERIC_BOARD_REV, "assembly.revision", },
>+ { "board.vendor", /* fab */ "assembly.vendor", },
>+ { "board.model", /* code name */ "assembly.model", },
>+};
>+
>+static int
>+nfp_devlink_versions_get_hwinfo(struct nfp_pf *pf, struct devlink_info_req *req)
>+{
>+ unsigned int i;
>+ int err;
>+
>+ for (i = 0; i < ARRAY_SIZE(nfp_devlink_versions_hwinfo); i++) {
>+ const struct nfp_devlink_versions_simple *info;
>+ const char *val;
>+
>+ info = &nfp_devlink_versions_hwinfo[i];
>+
>+ val = nfp_hwinfo_lookup(pf->hwinfo, info->hwinfo);
>+ if (!val)
>+ continue;
>+
>+ err = devlink_info_report_version(req, DEVLINK_VERSION_FIXED,
>+ info->key, val);
>+ if (err)
>+ return err;
>+ }
>+
>+ return 0;
>+}
>+
> static int
> nfp_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
> struct netlink_ext_ack *extack)
>@@ -191,7 +226,7 @@ nfp_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
> return err;
> }
>
>- return 0;
>+ return nfp_devlink_versions_get_hwinfo(pf, req);
> }
>
> const struct devlink_ops nfp_devlink_ops = {
>diff --git a/include/net/devlink.h b/include/net/devlink.h
>index ec72638aa47f..3d553cc6693d 100644
>--- a/include/net/devlink.h
>+++ b/include/net/devlink.h
>@@ -428,6 +428,11 @@ enum devlink_param_wol_types {
> .validate = _validate, \
> }
>
>+/* Part number, identifier of board design */
>+#define DEVLINK_VERSION_GENERIC_BOARD_ID "board.id"
>+/* Revision of board design */
>+#define DEVLINK_VERSION_GENERIC_BOARD_REV "board.rev"
Please have this as a separate patch, not part of "nfp".
>+
> enum devlink_version_type {
> DEVLINK_VERSION_FIXED,
> DEVLINK_VERSION_STORED,
>--
>2.19.2
>
^ permalink raw reply
* Re: [PATCH net-next v2 2/7] devlink: add version reporting to devlink info API
From: Jiri Pirko @ 2019-01-30 21:54 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, oss-drivers, andrew, f.fainelli, mkubecek, eugenem,
jonathan.lemon
In-Reply-To: <20190130190513.25718-3-jakub.kicinski@netronome.com>
Wed, Jan 30, 2019 at 08:05:08PM CET, jakub.kicinski@netronome.com wrote:
>ethtool -i has a few fixed-size fields which can be used to report
>firmware version and expansion ROM version. Unfortunately, modern
>hardware has more firmware components. There is usually some
>datapath microcode, management controller, PXE drivers, and a
>CPLD load. Running ethtool -i on modern controllers reveals the
>fact that vendors cram multiple values into firmware version field.
>
>Here are some examples from systems I could lay my hands on quickly:
>
>tg3: "FFV20.2.17 bc 5720-v1.39"
>i40e: "6.01 0x800034a4 1.1747.0"
>nfp: "0.0.3.5 0.25 sriov-2.1.16 nic"
>
>Add a new devlink API to allow retrieving multiple versions, and
>provide user-readable name for those versions.
>
>While at it break down the versions into three categories:
> - fixed - this is the board/fixed component version, usually vendors
> report information like the board version in the PCI VPD,
> but it will benefit from naming and common API as well;
> - running - this is the running firmware version;
> - stored - this is firmware in the flash, after firmware update
> this value will reflect the flashed version, while the
> running version may only be updated after reboot.
>
>RFCv2:
> - remove the nesting in attr DEVLINK_ATTR_INFO_VERSIONS (now
> versions are mixed with other info attrs)l
> - have the driver report versions from the same callback as
> other info.
>
>Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
>---
> include/net/devlink.h | 18 ++++++++++++++++
> include/uapi/linux/devlink.h | 5 +++++
> net/core/devlink.c | 40 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 63 insertions(+)
>
>diff --git a/include/net/devlink.h b/include/net/devlink.h
>index 5ef3570a3859..ec72638aa47f 100644
>--- a/include/net/devlink.h
>+++ b/include/net/devlink.h
>@@ -428,6 +428,12 @@ enum devlink_param_wol_types {
> .validate = _validate, \
> }
>
>+enum devlink_version_type {
devlink_info_version_type
>+ DEVLINK_VERSION_FIXED,
DEVLINK_INFO_VERSION_*
>+ DEVLINK_VERSION_STORED,
>+ DEVLINK_VERSION_RUNNING,
>+};
>+
> struct devlink_region;
> struct devlink_info_req;
>
>@@ -614,6 +620,10 @@ int devlink_info_report_serial_number(struct devlink_info_req *req,
> const char *sn);
> int devlink_info_report_driver_name(struct devlink_info_req *req,
> const char *name);
>+int devlink_info_report_version(struct devlink_info_req *req,
>+ enum devlink_version_type type,
>+ const char *version_name,
>+ const char *version_value);
>
> #else
>
>@@ -923,6 +933,14 @@ devlink_info_report_serial_number(struct devlink_info_req *req, const char *sn)
> {
> return 0;
> }
>+
>+static inline int
>+devlink_info_report_version(struct devlink_info_req *req,
>+ enum devlink_version_type type,
>+ const char *version_name, const char *version_value)
>+{
>+ return 0;
>+}
> #endif
>
> #endif /* _NET_DEVLINK_H_ */
>diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
>index fd089baa7c50..a61b87c73c3f 100644
>--- a/include/uapi/linux/devlink.h
>+++ b/include/uapi/linux/devlink.h
>@@ -294,6 +294,11 @@ enum devlink_attr {
>
> DEVLINK_ATTR_INFO_DRV_NAME, /* string */
> DEVLINK_ATTR_INFO_SERIAL_NUMBER, /* string */
>+ DEVLINK_ATTR_INFO_VERSION_FIXED, /* nested */
>+ DEVLINK_ATTR_INFO_VERSION_RUNNING, /* nested */
>+ DEVLINK_ATTR_INFO_VERSION_STORED, /* nested */
>+ DEVLINK_ATTR_INFO_VERSION_NAME, /* string */
>+ DEVLINK_ATTR_INFO_VERSION_VALUE, /* string */
>
> /* add new attributes above here, update the policy in devlink.c */
>
>diff --git a/net/core/devlink.c b/net/core/devlink.c
>index 1b941493fdff..e2027d3a5e40 100644
>--- a/net/core/devlink.c
>+++ b/net/core/devlink.c
>@@ -3732,6 +3732,46 @@ int devlink_info_report_serial_number(struct devlink_info_req *req,
> }
> EXPORT_SYMBOL_GPL(devlink_info_report_serial_number);
>
>+int devlink_info_report_version(struct devlink_info_req *req,
devlink_info_version_put()
>+ enum devlink_version_type type,
>+ const char *version_name,
>+ const char *version_value)
>+{
>+ static const enum devlink_attr type2attr[] = {
>+ [DEVLINK_VERSION_FIXED] = DEVLINK_ATTR_INFO_VERSION_FIXED,
>+ [DEVLINK_VERSION_STORED] = DEVLINK_ATTR_INFO_VERSION_STORED,
>+ [DEVLINK_VERSION_RUNNING] = DEVLINK_ATTR_INFO_VERSION_RUNNING,
I think it would be perhaps nice to have a set of wrappers:
devlink_info_version_fixed_put()
devlink_info_version_stored_put()
devlink_info_version_running_put()
And then have static devlink_info_version_put() which accepts ATTR value
directly. Then you can avoid the intermediate enum, array and checks.
>+ };
>+ struct nlattr *nest;
>+ int err;
>+
>+ if (type >= ARRAY_SIZE(type2attr) || !type2attr[type])
>+ return -EINVAL;
>+
>+ nest = nla_nest_start(req->msg, type2attr[type]);
>+ if (!nest)
>+ return -EMSGSIZE;
>+
>+ err = nla_put_string(req->msg, DEVLINK_ATTR_INFO_VERSION_NAME,
>+ version_name);
>+ if (err)
>+ goto nla_put_failure;
>+
>+ err = nla_put_string(req->msg, DEVLINK_ATTR_INFO_VERSION_VALUE,
>+ version_value);
>+ if (err)
>+ goto nla_put_failure;
>+
>+ nla_nest_end(req->msg, nest);
>+
>+ return 0;
>+
>+nla_put_failure:
>+ nla_nest_cancel(req->msg, nest);
>+ return err;
>+}
>+EXPORT_SYMBOL_GPL(devlink_info_report_version);
>+
> static int
> devlink_nl_info_fill(struct sk_buff *msg, struct devlink *devlink,
> enum devlink_command cmd, u32 portid,
>--
>2.19.2
>
^ permalink raw reply
* Re: [RFC 03/14] net: hstats: add basic/core functionality
From: David Ahern @ 2019-01-30 22:33 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, oss-drivers, netdev, jiri, f.fainelli, andrew, mkubecek,
simon.horman, jesse.brandeburg, maciejromanfijalkowski,
vasundhara-v.volam, michael.chan, shalomt, idosch
In-Reply-To: <20190130094444.204c46b7@cakuba.hsd1.ca.comcast.net>
On 1/30/19 10:44 AM, Jakub Kicinski wrote:
> On Tue, 29 Jan 2019 21:18:28 -0700, David Ahern wrote:
>> On 1/28/19 4:44 PM, Jakub Kicinski wrote:
>>> @@ -4946,6 +4964,9 @@ static size_t if_nlmsg_stats_size(const struct net_device *dev,
>>> rcu_read_unlock();
>>> }
>>>
>>> + if (stats_attr_valid(filter_mask, IFLA_STATS_LINK_HSTATS, 0))
>>
>> filter_mask is populated by RTEXT_FILTER_ from
>> include/uapi/linux/rtnetlink.h
>
> ext_filter_mask is from IFLA_EXT_MASK and has RTEXT_FILTER_ bits set.
> Here the mask is from struct if_stats_msg::filter_mask of RTM_GETSTATS.
> Am I missing the point? :S
nm. I confused the two filter_mask's.
>
>>> + size += rtnl_get_link_hstats_size(dev);
>>
>> rtnl_get_link_hstats_size == __rtnl_get_link_hstats can return < 0.
>
> Ups! Thank you!
>
> In general how much do you dislike this code? :)
>
Not having spent much time on the stats details it is hard to follow -
i.e, requires a fair bit of time iterative over the core patches.
^ permalink raw reply
* Re: [PATCH net-next v2 08/12] net: dsa: Add ndo_vlan_rx_{add,kill}_vid implementation
From: Vivien Didelot @ 2019-01-30 22:38 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev, Florian Fainelli, andrew, davem, idosch, jiri,
ilias.apalodimas, ivan.khoronzhuk, roopa, nikolay
In-Reply-To: <20190130005548.2212-9-f.fainelli@gmail.com>
Hi Florian,
On Tue, 29 Jan 2019 16:55:44 -0800, Florian Fainelli <f.fainelli@gmail.com> wrote:
> - if (br_vlan_enabled(dp->bridge_dev))
> + /* Can be called from dsa_slave_port_obj_add() or
> + * dsa_slave_vlan_rx_add_vid()
> + */
> + if ((dp->bridge_dev && br_vlan_enabled(dp->bridge_dev)) ||
> + !dp->bridge_dev)
if (!dp->bridge_dev || br_vlan_enabled(dp->bridge_dev))
This would be sufficient.
>
> return 0;
> @@ -270,7 +274,11 @@ int dsa_port_vlan_del(struct dsa_port *dp,
> if (netif_is_bridge_master(vlan->obj.orig_dev))
> return -EOPNOTSUPP;
>
> - if (br_vlan_enabled(dp->bridge_dev))
> + /* Can be called from dsa_slave_port_obj_del() or
> + * dsa_slave_vlan_rx_kill_vid()
> + */
> + if ((dp->bridge_dev && br_vlan_enabled(dp->bridge_dev)) ||
> + !dp->bridge_dev)
Same here.
Thanks,
Vivien
^ permalink raw reply
* Re: [PATCH net-next v2 10/12] net: dsa: Wire up multicast IGMP snooping attribute notification
From: Florian Fainelli @ 2019-01-30 23:02 UTC (permalink / raw)
To: Andrew Lunn
Cc: netdev, vivien.didelot, davem, idosch, jiri, ilias.apalodimas,
ivan.khoronzhuk, roopa, nikolay
In-Reply-To: <20190130224638.GC30115@lunn.ch>
On 1/30/19 2:46 PM, Andrew Lunn wrote:
>> @@ -425,6 +425,8 @@ struct dsa_switch_ops {
>> /*
>> * Multicast database
>> */
>> + int (*port_multicast_toggle)(struct dsa_switch *ds, int port,
>> + bool mc_disabled);
>> int (*port_mdb_prepare)(struct dsa_switch *ds, int port,
>> const struct switchdev_obj_port_mdb *mdb);
>> void (*port_mdb_add)(struct dsa_switch *ds, int port,
>
> Hi Florian
>
> I took a second look at this, after you asked the question.
>
> What i see above is that port_multicast_toggle is different to
> port_mdb_prepare and port_mdb_add. From this context, it looks like
> there should not be a tab between int and (*port_multicast_toggle).
>
> However, when you look at the real file, it becomes clear that it is
> actually normal to have a tab between the type and the name of the
> function pointer, and that port_mdb_prepare and port_mdb_add are
> different to the rest.
>
> So you patch is O.K.
No worries, thanks for taking a look!
--
Florian
^ permalink raw reply
* Re: [PATCH net-next 00/12] code optimizations & bugfixes for HNS3 driver
From: David Miller @ 2019-01-30 22:50 UTC (permalink / raw)
To: tanhuazhong
Cc: netdev, linux-kernel, huangdaode, yisen.zhuang, salil.mehta,
linuxarm
In-Reply-To: <20190130205552.8512-1-tanhuazhong@huawei.com>
From: Huazhong Tan <tanhuazhong@huawei.com>
Date: Thu, 31 Jan 2019 04:55:40 +0800
> This patchset includes bugfixes and code optimizations for the HNS3
> ethernet controller driver
Series applied, thanks.
^ 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