* Re: [PATCH net 1/2] net: dsa: mt7530: remove redundant MODULE_ALIAS entries
From: Andrew Lunn @ 2018-03-26 15:32 UTC (permalink / raw)
To: sean.wang
Cc: davem, f.fainelli, vivien.didelot, netdev, linux-kernel,
linux-mediatek
In-Reply-To: <2497321afc9156f7954e813721f60a0a95c03bd2.1522057659.git.sean.wang@mediatek.com>
On Mon, Mar 26, 2018 at 06:07:09PM +0800, sean.wang@mediatek.com wrote:
> From: Sean Wang <sean.wang@mediatek.com>
>
> MODULE_ALIAS exports information to allow the module to be auto-loaded at
> boot for the drivers registered using legacy platform registration.
>
> However, currently the driver is always used by DT-only platform,
> MODULE_ALIAS is redundant and should be removed properly.
>
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
For this, and the second patch:
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: linux-next on x60: network manager often complains "network is disabled" after resume
From: Dan Williams @ 2018-03-26 15:33 UTC (permalink / raw)
To: Pavel Machek
Cc: Woody Suwalski, Rafael J. Wysocki, kernel list,
Linux-pm mailing list, Netdev list
In-Reply-To: <20180325061927.GA2148@amd>
On Sun, 2018-03-25 at 08:19 +0200, Pavel Machek wrote:
> > > > Ok, what does 'nmcli dev' and 'nmcli radio' show?
> > >
> > > Broken state.
> > >
> > > pavel@amd:~$ nmcli dev
> > > DEVICE TYPE STATE CONNECTION
> > > eth1 ethernet unavailable --
> > > lo loopback unmanaged --
> > > wlan0 wifi unmanaged --
> >
> > If the state is "unmanaged" on resume, that would indicate a
> > problem
> > with sleep/wake and likely not a kernel network device issue.
> >
> > We should probably move this discussion to the NM lists to debug
> > further. Before you suspend, run "nmcli gen log level trace" to
> > turn
> > on full debug logging, then reproduce the issue, and send a pointer
> > to
> > those logs (scrubbed for anything you consider sensitive) to the NM
> > mailing list.
>
> Hmm :-)
>
> root@amd:/data/pavel# nmcli gen log level trace
> Error: Unknown log level 'trace'
What NM version? 'trace' is pretty old (since 1.0 from December 2014)
so unless you're using a really, really old version of Debian I'd
expect you'd have it. Anyway, debug would do.
> root@amd:/data/pavel# nmcli gen log level help
> Error: Unknown log level 'help'
nmcli gen help
> root@amd:/data/pavel# nmcli gen log level
> Error: value for 'level' argument is required.
> root@amd:/data/pavel# nmcli gen log level debug
This should be OK.
> root@amd:/data/pavel# cat /var/log/sys/log
It routes it to whatever the syslog 'daemon' facility logs to (however
that's configured on your system). Usually /var/log/messages or
/var/log/daemon.log or sometimes your distro configures it to
/var/log/NetworkManager.log.
Or if you're using a systemd-based distro, it would probably be in the
systemd journal so "journalctl -b -u NetworkManager"
> Where do I get the logs? I don't see much in the syslog...
> And.. It seems that it is "every other suspend". One resume results
> in
> broken network, one in working one, one in broken one...
Does your distro use pm-utils, upower, or systemd for suspend/resume
handling?
Dan
^ permalink raw reply
* [PATCH] ipv6: addrconf: Use normal debugging style
From: Joe Perches @ 2018-03-26 15:35 UTC (permalink / raw)
To: David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI; +Cc: netdev, linux-kernel
Remove local ADBG macro and use netdev_dbg/pr_debug
Miscellanea:
o Remove unnecessary debug message after allocation failure as there
already is a dump_stack() on the failure paths
o Leave the allocation failure message on snmp6_alloc_dev as there
is one code path that does not do a dump_stack()
Signed-off-by: Joe Perches <joe@perches.com>
---
Remerged resend of http://patchwork.ozlabs.org/patch/826028/
back in October 2017
net/ipv6/addrconf.c | 28 ++++++++--------------------
1 file changed, 8 insertions(+), 20 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 6fd4bbdc444f..0d104bd74934 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -94,15 +94,6 @@
#include <linux/seq_file.h>
#include <linux/export.h>
-/* Set to 3 to get tracing... */
-#define ACONF_DEBUG 2
-
-#if ACONF_DEBUG >= 3
-#define ADBG(fmt, ...) printk(fmt, ##__VA_ARGS__)
-#else
-#define ADBG(fmt, ...) do { if (0) printk(fmt, ##__VA_ARGS__); } while (0)
-#endif
-
#define INFINITY_LIFE_TIME 0xFFFFFFFF
#define IPV6_MAX_STRLEN \
@@ -409,9 +400,8 @@ static struct inet6_dev *ipv6_add_dev(struct net_device *dev)
dev_hold(dev);
if (snmp6_alloc_dev(ndev) < 0) {
- ADBG(KERN_WARNING
- "%s: cannot allocate memory for statistics; dev=%s.\n",
- __func__, dev->name);
+ netdev_dbg(dev, "%s: cannot allocate memory for statistics\n",
+ __func__);
neigh_parms_release(&nd_tbl, ndev->nd_parms);
dev_put(dev);
kfree(ndev);
@@ -419,9 +409,8 @@ static struct inet6_dev *ipv6_add_dev(struct net_device *dev)
}
if (snmp6_register_dev(ndev) < 0) {
- ADBG(KERN_WARNING
- "%s: cannot create /proc/net/dev_snmp6/%s\n",
- __func__, dev->name);
+ netdev_dbg(dev, "%s: cannot create /proc/net/dev_snmp6/%s\n",
+ __func__, dev->name);
goto err_release;
}
@@ -984,7 +973,7 @@ static int ipv6_add_addr_hash(struct net_device *dev, struct inet6_ifaddr *ifa)
/* Ignore adding duplicate addresses on an interface */
if (ipv6_chk_same_addr(dev_net(dev), &ifa->addr, dev, hash)) {
- ADBG("ipv6_add_addr: already assigned\n");
+ netdev_dbg(dev, "ipv6_add_addr: already assigned\n");
err = -EEXIST;
} else {
hlist_add_head_rcu(&ifa->addr_lst, &inet6_addr_lst[hash]);
@@ -1044,7 +1033,6 @@ ipv6_add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
ifa = kzalloc(sizeof(*ifa), gfp_flags);
if (!ifa) {
- ADBG("ipv6_add_addr: malloc failed\n");
err = -ENOBUFS;
goto out;
}
@@ -2618,7 +2606,7 @@ void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len, bool sllao)
pinfo = (struct prefix_info *) opt;
if (len < sizeof(struct prefix_info)) {
- ADBG("addrconf: prefix option too short\n");
+ netdev_dbg(dev, "addrconf: prefix option too short\n");
return;
}
@@ -4446,8 +4434,8 @@ static void addrconf_verify_rtnl(void)
if (time_before(next_sched, jiffies + ADDRCONF_TIMER_FUZZ_MAX))
next_sched = jiffies + ADDRCONF_TIMER_FUZZ_MAX;
- ADBG(KERN_DEBUG "now = %lu, schedule = %lu, rounded schedule = %lu => %lu\n",
- now, next, next_sec, next_sched);
+ pr_debug("now = %lu, schedule = %lu, rounded schedule = %lu => %lu\n",
+ now, next, next_sec, next_sched);
mod_delayed_work(addrconf_wq, &addr_chk_work, next_sched - now);
rcu_read_unlock_bh();
}
--
2.15.0
^ permalink raw reply related
* Re: [PATCH net-next v2 0/3] Drop NETDEV_UNREGISTER_FINAL
From: David Miller @ 2018-03-26 15:35 UTC (permalink / raw)
To: ktkhai
Cc: Michal.Kalderon, Ariel.Elior, dledford, jgg, benve, 1dgoodell,
daniel, jakub.kicinski, ast, edumazet, linux, john.fastabend,
brouer, dsahern, netdev
In-Reply-To: <152182320819.2767.12723786642825837986.stgit@localhost.localdomain>
From: Kirill Tkhai <ktkhai@virtuozzo.com>
Date: Fri, 23 Mar 2018 19:47:09 +0300
> This series drops unused NETDEV_UNREGISTER_FINAL
> after some preparations.
>
> v2: New patch [2/3]. Use switch() in [1/3].
>
> The first version was acked by Jason Gunthorpe,
> and [1/3] was acked by David Ahern.
>
> Since there are differences to v1, I haven't added
> Acked-by tags of people. It would be nice, if you
> fill OK to tag v2 too.
Series applied, thank you!
^ permalink raw reply
* Re: [PATCH v5 2/2] net: ethernet: nixge: Add support for National Instruments XGE netdev
From: David Miller @ 2018-03-26 15:38 UTC (permalink / raw)
To: mdf; +Cc: linux-kernel, devicetree, netdev, robh+dt, andrew, f.fainelli
In-Reply-To: <20180323204128.29783-2-mdf@kernel.org>
From: Moritz Fischer <mdf@kernel.org>
Date: Fri, 23 Mar 2018 13:41:28 -0700
> +static void nixge_hw_dma_bd_release(struct net_device *ndev)
> +{
> + int i;
> + struct nixge_priv *priv = netdev_priv(ndev);
Please order local variables from longest to shortest line (ie. reverse
christmas tree layout).
> +static int nixge_hw_dma_bd_init(struct net_device *ndev)
> +{
> + u32 cr;
> + int i;
> + struct sk_buff *skb;
> + struct nixge_priv *priv = netdev_priv(ndev);
Likewise.
> +static void __nixge_device_reset(struct nixge_priv *priv, off_t offset)
> +{
> + u32 status;
> + int err;
> + /* Reset Axi DMA. This would reset NIXGE Ethernet core as well.
> + * The reset process of Axi DMA takes a while to complete as all
> + * pending commands/transfers will be flushed or completed during
> + * this reset process.
> + */
Please put an empty line between the local variable declarations
and this comment.
^ permalink raw reply
* Re: [PATCH v2 iproute2-next 3/6] rdma: Add CM_ID resource tracking information
From: David Ahern @ 2018-03-26 15:40 UTC (permalink / raw)
To: Steve Wise; +Cc: leon, stephen, netdev, linux-rdma
In-Reply-To: <743dc7a5306f9b3368fcd4c143cdd822250444a6.1520020530.git.swise@opengridcomputing.com>
On 2/27/18 9:07 AM, Steve Wise wrote:
> Sample output:
>
> # rdma resource
> 2: cxgb4_0: pd 5 cq 2 qp 2 cm_id 3 mr 7
> 3: mlx4_0: pd 7 cq 3 qp 3 cm_id 3 mr 7
>
> # rdma resource show cm_id
> link cxgb4_0/- lqpn 0 qp-type RC state LISTEN ps TCP pid 30485 comm rping src-addr 0.0.0.0:7174
> link cxgb4_0/2 lqpn 1048 qp-type RC state CONNECT ps TCP pid 30503 comm rping src-addr 172.16.2.1:7174 dst-addr 172.16.2.1:38246
> link cxgb4_0/2 lqpn 1040 qp-type RC state CONNECT ps TCP pid 30498 comm rping src-addr 172.16.2.1:38246 dst-addr 172.16.2.1:7174
> link mlx4_0/- lqpn 0 qp-type RC state LISTEN ps TCP pid 30485 comm rping src-addr 0.0.0.0:7174
> link mlx4_0/1 lqpn 539 qp-type RC state CONNECT ps TCP pid 30494 comm rping src-addr 172.16.99.1:7174 dst-addr 172.16.99.1:43670
> link mlx4_0/1 lqpn 538 qp-type RC state CONNECT ps TCP pid 30492 comm rping src-addr 172.16.99.1:43670 dst-addr 172.16.99.1:7174
>
> # rdma resource show cm_id dst-port 7174
> link cxgb4_0/2 lqpn 1040 qp-type RC state CONNECT ps TCP pid 30498 comm rping src-addr 172.16.2.1:38246 dst-addr 172.16.2.1:7174
> link mlx4_0/1 lqpn 538 qp-type RC state CONNECT ps TCP pid 30492 comm rping src-addr 172.16.99.1:43670 dst-addr 172.16.99.1:7174
>
> Signed-off-by: Steve Wise <swise@opengridcomputing.com>
> ---
> rdma/rdma.h | 2 +
> rdma/res.c | 258 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> rdma/utils.c | 5 ++
> 3 files changed, 264 insertions(+), 1 deletion(-)
>
> diff --git a/rdma/rdma.h b/rdma/rdma.h
> index 5809f70..e55205b 100644
> --- a/rdma/rdma.h
> +++ b/rdma/rdma.h
> @@ -18,10 +18,12 @@
> #include <libmnl/libmnl.h>
> #include <rdma/rdma_netlink.h>
> #include <time.h>
> +#include <net/if_arp.h>
>
> #include "list.h"
> #include "utils.h"
> #include "json_writer.h"
> +#include <rdma/rdma_cma.h>
>
> #define pr_err(args...) fprintf(stderr, ##args)
> #define pr_out(args...) fprintf(stdout, ##args)
> diff --git a/rdma/res.c b/rdma/res.c
> index 62f5c54..1ef4f20 100644
> --- a/rdma/res.c
> +++ b/rdma/res.c
> @@ -16,9 +16,11 @@ static int res_help(struct rd *rd)
> {
> pr_out("Usage: %s resource\n", rd->filename);
> pr_out(" resource show [DEV]\n");
> - pr_out(" resource show [qp]\n");
> + pr_out(" resource show [qp|cm_id]\n");
> pr_out(" resource show qp link [DEV/PORT]\n");
> pr_out(" resource show qp link [DEV/PORT] [FILTER-NAME FILTER-VALUE]\n");
> + pr_out(" resource show cm_id link [DEV/PORT]\n");
> + pr_out(" resource show cm_id link [DEV/PORT] [FILTER-NAME FILTER-VALUE]\n");
> return 0;
> }
>
> @@ -433,6 +435,230 @@ static int res_qp_parse_cb(const struct nlmsghdr *nlh, void *data)
> return MNL_CB_OK;
> }
>
> +static void print_qp_type(struct rd *rd, uint32_t val)
> +{
> + if (rd->json_output)
> + jsonw_string_field(rd->jw, "qp-type",
> + qp_types_to_str(val));
> + else
> + pr_out("qp-type %s ", qp_types_to_str(val));
> +}
> +
> +static const char *cm_id_state_to_str(uint8_t idx)
> +{
> + static const char * const cm_id_states_str[] = { "IDLE", "ADDR_QUERY",
> + "ADDR_RESOLVED", "ROUTE_QUERY", "ROUTE_RESOLVED",
> + "CONNECT", "DISCONNECT",
> + "ADDR_BOUND", "LISTEN", "DEVICE_REMOVAL", "DESTROYING" };
> +
In general lines should stay under 80 columns. There are a few
exceptions to the rule (e.g., print strings), but most of the long lines
you have in this patch need to conform.
> @@ -457,11 +683,41 @@ filters qp_valid_filters[MAX_NUMBER_OF_FILTERS] = {{ .name = "link",
>
> RES_FUNC(res_qp, RDMA_NLDEV_CMD_RES_QP_GET, qp_valid_filters, false);
>
> +static const struct
> +filters cm_id_valid_filters[MAX_NUMBER_OF_FILTERS] = {{ .name = "link",
> + .is_number = false },
> + { .name = "lqpn",
> + .is_number = true },
> + { .name = "qp-type",
> + .is_number = false },
> + { .name = "state",
> + .is_number = false },
> + { .name = "ps",
> + .is_number = false },
> + { .name = "dev-type",
> + .is_number = false },
> + { .name = "transport-type",
> + .is_number = false },
> + { .name = "pid",
> + .is_number = true },
> + { .name = "src-addr",
> + .is_number = false },
> + { .name = "src-port",
> + .is_number = true },
> + { .name = "dst-addr",
> + .is_number = false },
> + { .name = "dst-port",
> + .is_number = true }};
> +
The above would be more readable as
static const
struct filters cm_id_valid_filters[MAX_NUMBER_OF_FILTERS] = {
{ .name = "link", .is_number = false },
{ .name = "lqpn", .is_number = true },
...
Definitely do not split between struct and filters.
^ permalink raw reply
* Re: [PATCH iproute2-next 2/2] rdma: Print net device name and index for RDMA device
From: David Ahern @ 2018-03-26 15:44 UTC (permalink / raw)
To: Leon Romanovsky; +Cc: netdev, RDMA mailing list, Stephen Hemminger
In-Reply-To: <20180326150933.GI1877@mtr-leonro.local>
On 3/26/18 9:09 AM, Leon Romanovsky wrote:
> On Mon, Mar 26, 2018 at 08:06:52AM -0600, David Ahern wrote:
>> On 3/26/18 2:28 AM, Leon Romanovsky wrote:
>>> diff --git a/rdma/include/uapi/rdma/rdma_netlink.h b/rdma/include/uapi/rdma/rdma_netlink.h
>>> index 9446a721..45474f13 100644
>>> --- a/rdma/include/uapi/rdma/rdma_netlink.h
>>> +++ b/rdma/include/uapi/rdma/rdma_netlink.h
>>> @@ -388,6 +388,10 @@ enum rdma_nldev_attr {
>>> RDMA_NLDEV_ATTR_RES_LOCAL_DMA_LKEY, /* u32 */
>>> RDMA_NLDEV_ATTR_RES_UNSAFE_GLOBAL_RKEY, /* u32 */
>>>
>>> + /* Netdev information for relevant protocols, like RoCE and iWARP */
>>> + RDMA_NLDEV_ATTR_NDEV_INDEX, /* u32 */
>>> + RDMA_NLDEV_ATTR_NDEV_NAME, /* string */
>>> +
>>> RDMA_NLDEV_ATTR_MAX
>>> };
>>> #endif /* _RDMA_NETLINK_H */
>>
>> Why wasn't the above included in patch 1 as part of the header file update?
>>
>>
>
> Kernel part is not accepted yet, this is why I separated them.
>
Please don't mix patches in a set like this. The entire set should be
consistent -- either all of them are in the kernel or all of them are
waiting to be committed.
^ permalink raw reply
* Re: [PATCH v5 bpf-next 06/10] tracepoint: compute num_args at build time
From: Alexei Starovoitov @ 2018-03-26 15:42 UTC (permalink / raw)
To: Mathieu Desnoyers, rostedt
Cc: David S. Miller, Daniel Borkmann, Linus Torvalds, Peter Zijlstra,
netdev, kernel-team, linux-api
In-Reply-To: <1787605856.4574.1522077244597.JavaMail.zimbra@efficios.com>
On 3/26/18 8:14 AM, Mathieu Desnoyers wrote:
> ----- On Mar 26, 2018, at 11:02 AM, rostedt rostedt@goodmis.org wrote:
>
>> On Fri, 23 Mar 2018 19:30:34 -0700
>> Alexei Starovoitov <ast@fb.com> wrote:
>>
>>> From: Alexei Starovoitov <ast@kernel.org>
>>>
>>> add fancy macro to compute number of arguments passed into tracepoint
>>> at compile time and store it as part of 'struct tracepoint'.
>>> The number is necessary to check safety of bpf program access that
>>> is coming in subsequent patch.
>>>
>>> for_each_tracepoint_range() api has no users inside the kernel.
>>> Make it more useful with ability to stop for_each() loop depending
>>> via callback return value.
>>> In such form it's used in subsequent patch.
>>
>> I believe this is used by LTTng.
>
> Indeed, and by SystemTAP as well.
>
> What justifies the need to stop mid-iteration ? A less intrusive alternative
> would be to use the "priv" data pointer to keep state telling further calls
> to return immediately. Does performance of iteration over tracepoints really
> matter here so much that stopping iteration immediately is worth it ?
I'm sure both you and Steven are not serious when you object
to _in-tree_ change to for_each_kernel_tracepoint() that
affects _out-of_tree_ modules?
Just change your module to 'return NULL' instead of plain 'return'.
^ permalink raw reply
* Re: [PATCH net 2/3] bonding: move dev_mc_sync after master_upper_dev_link in bond_enslave
From: Andy Gospodarek @ 2018-03-26 15:46 UTC (permalink / raw)
To: Xin Long
Cc: network dev, davem, Jiri Pirko, Wang Chen, Veaceslav Falico,
Nikolay Aleksandrov
In-Reply-To: <f988756adf856b3975e7941b9c7de593fed098e8.1521997984.git.lucien.xin@gmail.com>
On Mon, Mar 26, 2018 at 01:16:46AM +0800, Xin Long wrote:
> Beniamino found a crash when adding vlan as slave of bond which is also
> the parent link:
>
> ip link add bond1 type bond
> ip link set bond1 up
> ip link add link bond1 vlan1 type vlan id 80
> ip link set vlan1 master bond1
>
> The call trace is as below:
>
> [<ffffffffa850842a>] queued_spin_lock_slowpath+0xb/0xf
> [<ffffffffa8515680>] _raw_spin_lock+0x20/0x30
> [<ffffffffa83f6f07>] dev_mc_sync+0x37/0x80
> [<ffffffffc08687dc>] vlan_dev_set_rx_mode+0x1c/0x30 [8021q]
> [<ffffffffa83efd2a>] __dev_set_rx_mode+0x5a/0xa0
> [<ffffffffa83f7138>] dev_mc_sync_multiple+0x78/0x80
> [<ffffffffc084127c>] bond_enslave+0x67c/0x1190 [bonding]
> [<ffffffffa8401909>] do_setlink+0x9c9/0xe50
> [<ffffffffa8403bf2>] rtnl_newlink+0x522/0x880
> [<ffffffffa8403ff7>] rtnetlink_rcv_msg+0xa7/0x260
> [<ffffffffa8424ecb>] netlink_rcv_skb+0xab/0xc0
> [<ffffffffa83fe498>] rtnetlink_rcv+0x28/0x30
> [<ffffffffa8424850>] netlink_unicast+0x170/0x210
> [<ffffffffa8424bf8>] netlink_sendmsg+0x308/0x420
> [<ffffffffa83cc396>] sock_sendmsg+0xb6/0xf0
>
> This is actually a dead lock caused by sync slave hwaddr from master when
> the master is the slave's 'slave'. This dead loop check is actually done
> by netdev_master_upper_dev_link. However, Commit 1f718f0f4f97 ("bonding:
> populate neighbour's private on enslave") moved it after dev_mc_sync.
>
> This patch is to fix it by moving dev_mc_sync after master_upper_dev_link,
> so that this loop check would be earlier than dev_mc_sync. It also moves
> if (mode == BOND_MODE_8023AD) into if (!bond_uses_primary) clause as an
> improvement.
Nice optimization. :-)
>
> Note team driver also has this issue, I will fix it in another patch.
>
> Fixes: 1f718f0f4f97 ("bonding: populate neighbour's private on enslave")
> Reported-by: Beniamino Galvani <bgalvani@redhat.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
Acked-by: Andy Gospodarek <andy@greyhouse.net>
> ---
> drivers/net/bonding/bond_main.c | 73 ++++++++++++++++++++---------------------
> 1 file changed, 35 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 0c299de..55e1985 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -1528,44 +1528,11 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
> goto err_close;
> }
>
> - /* If the mode uses primary, then the following is handled by
> - * bond_change_active_slave().
> - */
> - if (!bond_uses_primary(bond)) {
> - /* set promiscuity level to new slave */
> - if (bond_dev->flags & IFF_PROMISC) {
> - res = dev_set_promiscuity(slave_dev, 1);
> - if (res)
> - goto err_close;
> - }
> -
> - /* set allmulti level to new slave */
> - if (bond_dev->flags & IFF_ALLMULTI) {
> - res = dev_set_allmulti(slave_dev, 1);
> - if (res)
> - goto err_close;
> - }
> -
> - netif_addr_lock_bh(bond_dev);
> -
> - dev_mc_sync_multiple(slave_dev, bond_dev);
> - dev_uc_sync_multiple(slave_dev, bond_dev);
> -
> - netif_addr_unlock_bh(bond_dev);
> - }
> -
> - if (BOND_MODE(bond) == BOND_MODE_8023AD) {
> - /* add lacpdu mc addr to mc list */
> - u8 lacpdu_multicast[ETH_ALEN] = MULTICAST_LACPDU_ADDR;
> -
> - dev_mc_add(slave_dev, lacpdu_multicast);
> - }
> -
> res = vlan_vids_add_by_dev(slave_dev, bond_dev);
> if (res) {
> netdev_err(bond_dev, "Couldn't add bond vlan ids to %s\n",
> slave_dev->name);
> - goto err_hwaddr_unsync;
> + goto err_close;
> }
>
> prev_slave = bond_last_slave(bond);
> @@ -1725,6 +1692,37 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
> goto err_upper_unlink;
> }
>
> + /* If the mode uses primary, then the following is handled by
> + * bond_change_active_slave().
> + */
> + if (!bond_uses_primary(bond)) {
> + /* set promiscuity level to new slave */
> + if (bond_dev->flags & IFF_PROMISC) {
> + res = dev_set_promiscuity(slave_dev, 1);
> + if (res)
> + goto err_sysfs_del;
> + }
> +
> + /* set allmulti level to new slave */
> + if (bond_dev->flags & IFF_ALLMULTI) {
> + res = dev_set_allmulti(slave_dev, 1);
> + if (res)
> + goto err_sysfs_del;
> + }
> +
> + netif_addr_lock_bh(bond_dev);
> + dev_mc_sync_multiple(slave_dev, bond_dev);
> + dev_uc_sync_multiple(slave_dev, bond_dev);
> + netif_addr_unlock_bh(bond_dev);
> +
> + if (BOND_MODE(bond) == BOND_MODE_8023AD) {
> + /* add lacpdu mc addr to mc list */
> + u8 lacpdu_multicast[ETH_ALEN] = MULTICAST_LACPDU_ADDR;
> +
> + dev_mc_add(slave_dev, lacpdu_multicast);
> + }
> + }
> +
> bond->slave_cnt++;
> bond_compute_features(bond);
> bond_set_carrier(bond);
> @@ -1748,6 +1746,9 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
> return 0;
>
> /* Undo stages on error */
> +err_sysfs_del:
> + bond_sysfs_slave_del(new_slave);
> +
> err_upper_unlink:
> bond_upper_dev_unlink(bond, new_slave);
>
> @@ -1768,10 +1769,6 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
> synchronize_rcu();
> slave_disable_netpoll(new_slave);
>
> -err_hwaddr_unsync:
> - if (!bond_uses_primary(bond))
> - bond_hw_addr_flush(bond_dev, slave_dev);
> -
> err_close:
> slave_dev->priv_flags &= ~IFF_BONDING;
> dev_close(slave_dev);
^ permalink raw reply
* Re: [PATCH v5 bpf-next 00/10] bpf, tracing: introduce bpf raw tracepoints
From: Steven Rostedt @ 2018-03-26 15:47 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Alexei Starovoitov, davem, torvalds, peterz, netdev, kernel-team,
linux-api
In-Reply-To: <ba96cda5-c4d6-4d66-0128-b7053d25fcd8@iogearbox.net>
On Mon, 26 Mar 2018 17:32:02 +0200
Daniel Borkmann <daniel@iogearbox.net> wrote:
> On 03/26/2018 05:04 PM, Steven Rostedt wrote:
> > On Mon, 26 Mar 2018 10:28:03 +0200
> > Daniel Borkmann <daniel@iogearbox.net> wrote:
> >
> >>> tracepoint base kprobe+bpf tracepoint+bpf raw_tracepoint+bpf
> >>> task_rename 1.1M 769K 947K 1.0M
> >>> urandom_read 789K 697K 750K 755K
> >>
> >> Applied to bpf-next, thanks Alexei!
> >
> > Please wait till you have the proper acks. Some of this affects
> > tracing.
>
> Ok, I thought time up to v5 was long enough. Anyway, in case there are
> objections I can still toss out the series from bpf-next tree worst case
> should e.g. follow-up fixups not be appropriate.
Yeah, I've been traveling a bit which slowed down my review process
(trying to catch up). My main concern is with patch 6, as there are
external users of those functions. Although, we generally don't cater
to out of tree code, we play nice with LTTng, and I don't want to break
it.
I also should probably pull in the patches and run them through my
tests to make sure they don't have any other side effects.
-- Steve
^ permalink raw reply
* Re: [PATCH v2] of_net: Implement of_get_nvmem_mac_address helper
From: Andrew Lunn @ 2018-03-26 15:50 UTC (permalink / raw)
To: Mike Looijmans
Cc: netdev, linux-kernel, devicetree, f.fainelli, robh+dt,
frowand.list
In-Reply-To: <1522046489-19652-1-git-send-email-mike.looijmans@topic.nl>
On Mon, Mar 26, 2018 at 08:41:29AM +0200, Mike Looijmans wrote:
> It's common practice to store MAC addresses for network interfaces into
> nvmem devices. However the code to actually do this in the kernel lacks,
> so this patch adds of_get_nvmem_mac_address() for drivers to obtain the
> address from an nvmem cell provider.
>
> This is particulary useful on devices where the ethernet interface cannot
> be configured by the bootloader, for example because it's in an FPGA.
>
> Tested by adapting the cadence macb driver to call this instead of
> of_get_mac_address().
Hi Mike
I can understand you not wanting to modify all the call sites for
of_get_mac_address().
However, the name of_get_nvmem_mac_address() suggests it gets the MAC
address from NVMEM. I think people are going to be surprised when they
find it first tries for a MAC address directly in device tree. I would
drop the call to of_get_mac_address(), and have the MAC driver call
both.
You could also maybe take a look at fwnode_get_mac_address(). It
should work for both OF and ACPI. It fits better because is passes a
char * for the address. You could make that do both, and call it from
the macb driver. dev_fwnode() probably does what you want.
Andrew
^ permalink raw reply
* Re: [PATCH v5 bpf-next 06/10] tracepoint: compute num_args at build time
From: Mathieu Desnoyers @ 2018-03-26 15:55 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: rostedt, David S. Miller, Daniel Borkmann, Linus Torvalds,
Peter Zijlstra, netdev, kernel-team, linux-api, Frank Ch. Eigler
In-Reply-To: <5bcacdb5-e72f-b67a-4884-61fcedf0938a@fb.com>
----- On Mar 26, 2018, at 11:42 AM, Alexei Starovoitov ast@fb.com wrote:
> On 3/26/18 8:14 AM, Mathieu Desnoyers wrote:
>> ----- On Mar 26, 2018, at 11:02 AM, rostedt rostedt@goodmis.org wrote:
>>
>>> On Fri, 23 Mar 2018 19:30:34 -0700
>>> Alexei Starovoitov <ast@fb.com> wrote:
>>>
>>>> From: Alexei Starovoitov <ast@kernel.org>
>>>>
>>>> add fancy macro to compute number of arguments passed into tracepoint
>>>> at compile time and store it as part of 'struct tracepoint'.
>>>> The number is necessary to check safety of bpf program access that
>>>> is coming in subsequent patch.
>>>>
>>>> for_each_tracepoint_range() api has no users inside the kernel.
>>>> Make it more useful with ability to stop for_each() loop depending
>>>> via callback return value.
>>>> In such form it's used in subsequent patch.
>>>
>>> I believe this is used by LTTng.
>>
>> Indeed, and by SystemTAP as well.
>>
>> What justifies the need to stop mid-iteration ? A less intrusive alternative
>> would be to use the "priv" data pointer to keep state telling further calls
>> to return immediately. Does performance of iteration over tracepoints really
>> matter here so much that stopping iteration immediately is worth it ?
>
> I'm sure both you and Steven are not serious when you object
> to _in-tree_ change to for_each_kernel_tracepoint() that
> affects _out-of_tree_ modules?
>
> Just change your module to 'return NULL' instead of plain 'return'.
I never said I objected to adapt the LTTng out of tree code. If there is a
solid reason for changing the kernel API, I will adapt my code to those
changes.
What I'm trying to understand here is whether there is solid ground for
the added complexity you are proposing. Is it a performance enhancement ?
If so, explanation of the use cases targeted, and numbers that measure
performance improvements are needed.
How is your patch making tracepoints "more useful" ?
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* Re: [PATCH v5 bpf-next 06/10] tracepoint: compute num_args at build time
From: Steven Rostedt @ 2018-03-26 15:56 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: davem, daniel, torvalds, peterz, netdev, kernel-team, linux-api
In-Reply-To: <20180324023038.938665-7-ast@fb.com>
On Fri, 23 Mar 2018 19:30:34 -0700
Alexei Starovoitov <ast@fb.com> wrote:
> +static void *for_each_tracepoint_range(struct tracepoint * const *begin,
> + struct tracepoint * const *end,
> + void *(*fct)(struct tracepoint *tp, void *priv),
> + void *priv)
> {
> struct tracepoint * const *iter;
> + void *ret;
>
> if (!begin)
> - return;
> - for (iter = begin; iter < end; iter++)
> - fct(*iter, priv);
> + return NULL;
> + for (iter = begin; iter < end; iter++) {
> + ret = fct(*iter, priv);
> + if (ret)
> + return ret;
So you just stopped the loop here. You have an inconsistent state. What
about the functions that were called before. How do you undo them? Or
what about the rest that haven't been touched. This function gives no
feedback to the caller.
-- Steve
> + }
> + return NULL;
> }
^ permalink raw reply
* Re: [PATCH v5 2/2] net: ethernet: nixge: Add support for National Instruments XGE netdev
From: Moritz Fischer @ 2018-03-26 15:56 UTC (permalink / raw)
To: David Miller
Cc: mdf, linux-kernel, devicetree, netdev, robh+dt, andrew,
f.fainelli
In-Reply-To: <20180326.113830.932262386304702367.davem@davemloft.net>
Hi David,
On Mon, Mar 26, 2018 at 11:38:30AM -0400, David Miller wrote:
> From: Moritz Fischer <mdf@kernel.org>
> Date: Fri, 23 Mar 2018 13:41:28 -0700
>
> > +static void nixge_hw_dma_bd_release(struct net_device *ndev)
> > +{
> > + int i;
> > + struct nixge_priv *priv = netdev_priv(ndev);
>
> Please order local variables from longest to shortest line (ie. reverse
> christmas tree layout).
Sure.
>
> > +static int nixge_hw_dma_bd_init(struct net_device *ndev)
> > +{
> > + u32 cr;
> > + int i;
> > + struct sk_buff *skb;
> > + struct nixge_priv *priv = netdev_priv(ndev);
>
> Likewise.
Sure.
>
> > +static void __nixge_device_reset(struct nixge_priv *priv, off_t offset)
> > +{
> > + u32 status;
> > + int err;
> > + /* Reset Axi DMA. This would reset NIXGE Ethernet core as well.
> > + * The reset process of Axi DMA takes a while to complete as all
> > + * pending commands/transfers will be flushed or completed during
> > + * this reset process.
> > + */
>
> Please put an empty line between the local variable declarations
> and this comment.
Will do in v6
Thanks for your review!
Cheers Moritz
^ permalink raw reply
* [PATCH bpf-next]: add sock_ops R/W access to ipv4 tos
From: Nikita V. Shirokov @ 2018-03-26 15:36 UTC (permalink / raw)
To: brakmo, ast, daniel, netdev; +Cc: kernel-team, tehnerd
bpf: Add sock_ops R/W access to ipv4 tos
Sample usage for tos:
bpf_getsockopt(skops, SOL_IP, IP_TOS, &v, sizeof(v))
where skops is a pointer to the ctx (struct bpf_sock_ops).
Signed-off-by: Nikita V. Shirokov <tehnerd@fb.com>
---
net/core/filter.c | 35 +++++++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/net/core/filter.c b/net/core/filter.c
index 00c711c..afd8255 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -3462,6 +3462,27 @@ BPF_CALL_5(bpf_setsockopt, struct bpf_sock_ops_kern *, bpf_sock,
ret = -EINVAL;
}
#ifdef CONFIG_INET
+ } else if (level == SOL_IP) {
+ if (optlen != sizeof(int) || sk->sk_family != AF_INET)
+ return -EINVAL;
+
+ val = *((int *)optval);
+ /* Only some options are supported */
+ switch (optname) {
+ case IP_TOS:
+ if (val < -1 || val > 0xff) {
+ ret = -EINVAL;
+ } else {
+ struct inet_sock *inet = inet_sk(sk);
+
+ if (val == -1)
+ val = 0;
+ inet->tos = val;
+ }
+ break;
+ default:
+ ret = -EINVAL;
+ }
#if IS_ENABLED(CONFIG_IPV6)
} else if (level == SOL_IPV6) {
if (optlen != sizeof(int) || sk->sk_family != AF_INET6)
@@ -3561,6 +3582,20 @@ BPF_CALL_5(bpf_getsockopt, struct bpf_sock_ops_kern *, bpf_sock,
} else {
goto err_clear;
}
+ } else if (level == SOL_IP) {
+ struct inet_sock *inet = inet_sk(sk);
+
+ if (optlen != sizeof(int) || sk->sk_family != AF_INET)
+ goto err_clear;
+
+ /* Only some options are supported */
+ switch (optname) {
+ case IP_TOS:
+ *((int *)optval) = (int)inet->tos;
+ break;
+ default:
+ goto err_clear;
+ }
#if IS_ENABLED(CONFIG_IPV6)
} else if (level == SOL_IPV6) {
struct ipv6_pinfo *np = inet6_sk(sk);
--
2.9.5
^ permalink raw reply related
* Re: [PATCH v5 bpf-next 00/10] bpf, tracing: introduce bpf raw tracepoints
From: Alexei Starovoitov @ 2018-03-26 16:00 UTC (permalink / raw)
To: Steven Rostedt, Daniel Borkmann
Cc: davem, torvalds, peterz, netdev, kernel-team, linux-api
In-Reply-To: <20180326114725.1999288a@gandalf.local.home>
On 3/26/18 8:47 AM, Steven Rostedt wrote:
> On Mon, 26 Mar 2018 17:32:02 +0200
> Daniel Borkmann <daniel@iogearbox.net> wrote:
>
>> On 03/26/2018 05:04 PM, Steven Rostedt wrote:
>>> On Mon, 26 Mar 2018 10:28:03 +0200
>>> Daniel Borkmann <daniel@iogearbox.net> wrote:
>>>
>>>>> tracepoint base kprobe+bpf tracepoint+bpf raw_tracepoint+bpf
>>>>> task_rename 1.1M 769K 947K 1.0M
>>>>> urandom_read 789K 697K 750K 755K
>>>>
>>>> Applied to bpf-next, thanks Alexei!
>>>
>>> Please wait till you have the proper acks. Some of this affects
>>> tracing.
>>
>> Ok, I thought time up to v5 was long enough. Anyway, in case there are
>> objections I can still toss out the series from bpf-next tree worst case
>> should e.g. follow-up fixups not be appropriate.
>
> Yeah, I've been traveling a bit which slowed down my review process
> (trying to catch up).
v1 of this set was posted Feb 28.
imo one month is not an acceptable delay for maintainer to review
the patches. You really need to consider group maintainership as
we do with Daniel for bpf tree.
> My main concern is with patch 6, as there are
> external users of those functions. Although, we generally don't cater
> to out of tree code, we play nice with LTTng, and I don't want to break
> it.
out-of-tree module is out of tree. I'm beyond surprised that you
propose to keep for_each_kernel_tracepoint() as-is with zero in-tree
users in order to keep lttng working.
> I also should probably pull in the patches and run them through my
> tests to make sure they don't have any other side effects.
so let me rephrase.
You're saying that a change to a function with zero in-tree users
can somehow break your tests?
How is that possible?
Does it mean you also have some out-of-tree modules that will break?
and that _is_ the real reason for objection?
^ permalink raw reply
* Re: [PATCH net 3/3] bonding: process the err returned by dev_set_allmulti properly in bond_enslave
From: Andy Gospodarek @ 2018-03-26 16:07 UTC (permalink / raw)
To: Xin Long
Cc: network dev, davem, Jiri Pirko, Wang Chen, Veaceslav Falico,
Nikolay Aleksandrov
In-Reply-To: <5695ce418ff6d7a2755053d6c7c35dfd3ebf2d6b.1521997984.git.lucien.xin@gmail.com>
On Mon, Mar 26, 2018 at 01:16:47AM +0800, Xin Long wrote:
> When dev_set_promiscuity(1) succeeds but dev_set_allmulti(1) fails,
> dev_set_promiscuity(-1) should be done before going to the err path.
> Otherwise, dev->promiscuity will leak.
>
> Fixes: 7e1a1ac1fbaa ("bonding: Check return of dev_set_promiscuity/allmulti")
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
Acked-by: Andy Gospodarek <andy@greyhouse.net>
> ---
> drivers/net/bonding/bond_main.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 55e1985..b7b1130 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -1706,8 +1706,11 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
> /* set allmulti level to new slave */
> if (bond_dev->flags & IFF_ALLMULTI) {
> res = dev_set_allmulti(slave_dev, 1);
> - if (res)
> + if (res) {
> + if (bond_dev->flags & IFF_PROMISC)
> + dev_set_promiscuity(slave_dev, -1);
> goto err_sysfs_del;
> + }
> }
>
> netif_addr_lock_bh(bond_dev);
^ permalink raw reply
* Re: [RFC PATCH 00/24] Introducing AF_XDP support
From: William Tu @ 2018-03-26 16:06 UTC (permalink / raw)
To: Björn Töpel
Cc: magnus.karlsson, Alexander Duyck, Alexander Duyck, John Fastabend,
Alexei Starovoitov, Jesper Dangaard Brouer, willemdebruijn.kernel,
Daniel Borkmann, Linux Kernel Network Developers,
Björn Töpel, michael.lundkvist, jesse.brandeburg,
anjali.singhai, jeffrey.b.shaw, ferruh.yigit, qi.z.zhang
In-Reply-To: <20180131135356.19134-1-bjorn.topel@gmail.com>
On Wed, Jan 31, 2018 at 5:53 AM, Björn Töpel <bjorn.topel@gmail.com> wrote:
> From: Björn Töpel <bjorn.topel@intel.com>
>
> This RFC introduces a new address family called AF_XDP that is
> optimized for high performance packet processing and zero-copy
> semantics. Throughput improvements can be up to 20x compared to V2 and
> V3 for the micro benchmarks included. Would be great to get your
> feedback on it. Note that this is the follow up RFC to AF_PACKET V4
> from November last year. The feedback from that RFC submission and the
> presentation at NetdevConf in Seoul was to create a new address family
> instead of building on top of AF_PACKET. AF_XDP is this new address
> family.
>
> The main difference between AF_XDP and AF_PACKET V2/V3 on a descriptor
> level is that TX and RX descriptors are separated from packet
> buffers. An RX or TX descriptor points to a data buffer in a packet
> buffer area. RX and TX can share the same packet buffer so that a
> packet does not have to be copied between RX and TX. Moreover, if a
> packet needs to be kept for a while due to a possible retransmit, then
> the descriptor that points to that packet buffer can be changed to
> point to another buffer and reused right away. This again avoids
> copying data.
>
> The RX and TX descriptor rings are registered with the setsockopts
> XDP_RX_RING and XDP_TX_RING, similar to AF_PACKET. The packet buffer
> area is allocated by user space and registered with the kernel using
> the new XDP_MEM_REG setsockopt. All these three areas are shared
> between user space and kernel space. The socket is then bound with a
> bind() call to a device and a specific queue id on that device, and it
> is not until bind is completed that traffic starts to flow.
>
> An XDP program can be loaded to direct part of the traffic on that
> device and queue id to user space through a new redirect action in an
> XDP program called bpf_xdpsk_redirect that redirects a packet up to
> the socket in user space. All the other XDP actions work just as
> before. Note that the current RFC requires the user to load an XDP
> program to get any traffic to user space (for example all traffic to
> user space with the one-liner program "return
> bpf_xdpsk_redirect();"). We plan on introducing a patch that removes
> this requirement and sends all traffic from a queue to user space if
> an AF_XDP socket is bound to it.
>
> AF_XDP can operate in three different modes: XDP_SKB, XDP_DRV, and
> XDP_DRV_ZC (shorthand for XDP_DRV with a zero-copy allocator as there
> is no specific mode called XDP_DRV_ZC). If the driver does not have
> support for XDP, or XDP_SKB is explicitly chosen when loading the XDP
> program, XDP_SKB mode is employed that uses SKBs together with the
> generic XDP support and copies out the data to user space. A fallback
> mode that works for any network device. On the other hand, if the
> driver has support for XDP (all three NDOs: ndo_bpf, ndo_xdp_xmit and
> ndo_xdp_flush), these NDOs, without any modifications, will be used by
> the AF_XDP code to provide better performance, but there is still a
> copy of the data into user space. The last mode, XDP_DRV_ZC, is XDP
> driver support with the zero-copy user space allocator that provides
> even better performance. In this mode, the networking HW (or SW driver
> if it is a virtual driver like veth) DMAs/puts packets straight into
> the packet buffer that is shared between user space and kernel
> space. The RX and TX descriptor queues of the networking HW are NOT
> shared to user space. Only the kernel can read and write these and it
> is the kernel driver's responsibility to translate these HW specific
> descriptors to the HW agnostic ones in the virtual descriptor rings
> that user space sees. This way, a malicious user space program cannot
> mess with the networking HW. This mode though requires some extensions
> to XDP.
>
> To get the XDP_DRV_ZC mode to work for RX, we chose to introduce a
> buffer pool concept so that the same XDP driver code can be used for
> buffers allocated using the page allocator (XDP_DRV), the user-space
> zero-copy allocator (XDP_DRV_ZC), or some internal driver specific
> allocator/cache/recycling mechanism. The ndo_bpf call has also been
> extended with two commands for registering and unregistering an XSK
> socket and is in the RX case mainly used to communicate some
> information about the user-space buffer pool to the driver.
>
> For the TX path, our plan was to use ndo_xdp_xmit and ndo_xdp_flush,
> but we run into problems with this (further discussion in the
> challenges section) and had to introduce a new NDO called
> ndo_xdp_xmit_xsk (xsk = XDP socket). It takes a pointer to a netdevice
> and an explicit queue id that packets should be sent out on. In
> contrast to ndo_xdp_xmit, it is asynchronous and pulls packets to be
> sent from the xdp socket (associated with the dev and queue
> combination that was provided with the NDO call) using a callback
> (get_tx_packet), and when they have been transmitted it uses another
> callback (tx_completion) to signal completion of packets. These
> callbacks are set via ndo_bpf in the new XDP_REGISTER_XSK
> command. ndo_xdp_xmit_xsk is exclusively used by the XDP socket code
> and thus does not clash with the XDP_REDIRECT use of
> ndo_xdp_xmit. This is one of the reasons that the XDP_DRV mode
> (without ZC) is currently not supported by TX. Please have a look at
> the challenges section for further discussions.
>
> The AF_XDP bind call acts on a queue pair (channel in ethtool speak),
> so the user needs to steer the traffic to the zero-copy enabled queue
> pair. Which queue to use, is up to the user.
>
> For an untrusted application, HW packet steering to a specific queue
> pair (the one associated with the application) is a requirement, as
> the application would otherwise be able to see other user space
> processes' packets. If the HW cannot support the required packet
> steering, XDP_DRV or XDP_SKB mode have to be used as they do not
> expose the NIC's packet buffer into user space as the packets are
> copied into user space from the NIC's packet buffer in the kernel.
>
> There is a xdpsock benchmarking/test application included. Say that
> you would like your UDP traffic from port 4242 to end up in queue 16,
> that we will enable AF_XDP on. Here, we use ethtool for this:
>
> ethtool -N p3p2 rx-flow-hash udp4 fn
> ethtool -N p3p2 flow-type udp4 src-port 4242 dst-port 4242 \
> action 16
>
> Running the l2fwd benchmark in XDP_DRV_ZC mode can then be done using:
>
> samples/bpf/xdpsock -i p3p2 -q 16 -l -N
>
> For XDP_SKB mode, use the switch "-S" instead of "-N" and all options
> can be displayed with "-h", as usual.
>
> We have run some benchmarks on a dual socket system with two Broadwell
> E5 2660 @ 2.0 GHz with hyperthreading turned off. Each socket has 14
> cores which gives a total of 28, but only two cores are used in these
> experiments. One for TR/RX and one for the user space application. The
> memory is DDR4 @ 2133 MT/s (1067 MHz) and the size of each DIMM is
> 8192MB and with 8 of those DIMMs in the system we have 64 GB of total
> memory. The compiler used is gcc version 5.4.0 20160609. The NIC is an
> Intel I40E 40Gbit/s using the i40e driver.
>
> Below are the results in Mpps of the I40E NIC benchmark runs for 64
> byte packets, generated by commercial packet generator HW that is
> generating packets at full 40 Gbit/s line rate.
>
> XDP baseline numbers without this RFC:
> xdp_rxq_info --action XDP_DROP 31.3 Mpps
> xdp_rxq_info --action XDP_TX 16.7 Mpps
>
> XDP performance with this RFC i.e. with the buffer allocator:
> XDP_DROP 21.0 Mpps
> XDP_TX 11.9 Mpps
>
> AF_PACKET V4 performance from previous RFC on 4.14-rc7:
> Benchmark V2 V3 V4 V4+ZC
> rxdrop 0.67 0.73 0.74 33.7
> txpush 0.98 0.98 0.91 19.6
> l2fwd 0.66 0.71 0.67 15.5
>
> AF_XDP performance:
> Benchmark XDP_SKB XDP_DRV XDP_DRV_ZC (all in Mpps)
> rxdrop 3.3 11.6 16.9
> txpush 2.2 NA* 21.8
> l2fwd 1.7 NA* 10.4
>
Hi,
I also did an evaluation of AF_XDP, however the performance isn't as
good as above.
I'd like to share the result and see if there are some tuning suggestions.
System:
16 core, Intel(R) Xeon(R) CPU E5-2440 v2 @ 1.90GHz
Intel 10G X540-AT2 ---> so I can only run XDP_SKB mode
AF_XDP performance:
Benchmark XDP_SKB
rxdrop 1.27 Mpps
txpush 0.99 Mpps
l2fwd 0.85 Mpps
NIC configuration:
the command
"ethtool -N p3p2 flow-type udp4 src-port 4242 dst-port 4242 action 16"
doesn't work on my ixgbe driver, so I use ntuple:
ethtool -K enp10s0f0 ntuple on
ethtool -U enp10s0f0 flow-type udp4 src-ip 10.1.1.100 action 1
then
echo 1 > /proc/sys/net/core/bpf_jit_enable
./xdpsock -i enp10s0f0 -r -S --queue=1
I also take a look at perf result:
For rxdrop:
86.56% xdpsock xdpsock [.] main
9.22% xdpsock [kernel.vmlinux] [k] nmi
4.23% xdpsock xdpsock [.] xq_enq
For l2fwd:
20.81% xdpsock xdpsock [.] main
10.64% xdpsock [kernel.vmlinux] [k] clflush_cache_range
8.46% xdpsock [kernel.vmlinux] [k] xsk_sendmsg
6.72% xdpsock [kernel.vmlinux] [k] skb_set_owner_w
5.89% xdpsock [kernel.vmlinux] [k] __domain_mapping
5.74% xdpsock [kernel.vmlinux] [k] alloc_skb_with_frags
4.62% xdpsock [kernel.vmlinux] [k] netif_skb_features
3.96% xdpsock [kernel.vmlinux] [k] ___slab_alloc
3.18% xdpsock [kernel.vmlinux] [k] nmi
I observed that the i40e's XDP_SKB result is much better than my ixgbe's result.
I wonder in XDP_SKB mode, does the driver make performance difference?
Or my cpu (E5-2440 v2 @ 1.90GHz) is too old?
Thanks
William
^ permalink raw reply
* Re: [PATCH 1/4] ethernet: Use octal not symbolic permissions
From: David Miller @ 2018-03-26 16:08 UTC (permalink / raw)
To: joe; +Cc: netdev
In-Reply-To: <31c47a8bb2af3f01178d83e890d2600f049ad709.1521845235.git.joe@perches.com>
Applied.
^ permalink raw reply
* Re: [PATCH 3/4] net: Use octal not symbolic permissions
From: David Miller @ 2018-03-26 16:08 UTC (permalink / raw)
To: joe
Cc: linux-wireless, jlayton, trond.myklebust, dhowells, linux-sctp,
linux-afs, steffen.klassert, herbert, sage, bridge, jchapman,
coreteam, kadlec, kuznet, idryomov, pablo, johan.hedberg, marcel,
linux-can, mkl, linux-hams, ceph-devel, linux-arm-kernel, bfields,
linux-nfs, linux-x25, nhorman, yoshfuji, socketcan, vyasevich,
linux-decnet-user, fw, linux-kernel, ralf, afaerbe
In-Reply-To: <738a74ff57f993d6b8f249425673413672d3fe70.1521845242.git.joe@perches.com>
Applied.
^ permalink raw reply
* Re: [PATCH 4/4] drivers/net: Use octal not symbolic permissions
From: David Miller @ 2018-03-26 16:08 UTC (permalink / raw)
To: joe
Cc: j.vosburgh, vfalico, andy, dmitry.tarnyagin, wg, mkl,
nicolas.ferre, alexandre.belloni, jpr, kys, haiyangz, sthemmin,
alex.aring, stefan, andrew, f.fainelli, paulus, mostrows, oliver,
wei.liu2, paul.durrant, boris.ostrovsky, jgross, netdev,
linux-kernel, linux-can, linux-arm-kernel, linux-hams, devel,
linux-wpan, linux-ppp, linux-usb, xen-devel
In-Reply-To: <8f24a711e6b8ec7b41356c378140fb54d510205c.1521845248.git.joe@perches.com>
From: Joe Perches <joe@perches.com>
Date: Fri, 23 Mar 2018 15:54:39 -0700
> Prefer the direct use of octal for permissions.
>
> Done with checkpatch -f --types=SYMBOLIC_PERMS --fix-inplace
> and some typing.
>
> Miscellanea:
>
> o Whitespace neatening around these conversions.
>
> Signed-off-by: Joe Perches <joe@perches.com>
Applied.
^ permalink raw reply
* Re: [PATCH v5 bpf-next 06/10] tracepoint: compute num_args at build time
From: Alexei Starovoitov @ 2018-03-26 16:08 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: rostedt, David S. Miller, Daniel Borkmann, Linus Torvalds,
Peter Zijlstra, netdev, kernel-team, linux-api, Frank Ch. Eigler
In-Reply-To: <523311773.184.1522079745421.JavaMail.zimbra@efficios.com>
On 3/26/18 8:55 AM, Mathieu Desnoyers wrote:
> ----- On Mar 26, 2018, at 11:42 AM, Alexei Starovoitov ast@fb.com wrote:
>
>> On 3/26/18 8:14 AM, Mathieu Desnoyers wrote:
>>> ----- On Mar 26, 2018, at 11:02 AM, rostedt rostedt@goodmis.org wrote:
>>>
>>>> On Fri, 23 Mar 2018 19:30:34 -0700
>>>> Alexei Starovoitov <ast@fb.com> wrote:
>>>>
>>>>> From: Alexei Starovoitov <ast@kernel.org>
>>>>>
>>>>> add fancy macro to compute number of arguments passed into tracepoint
>>>>> at compile time and store it as part of 'struct tracepoint'.
>>>>> The number is necessary to check safety of bpf program access that
>>>>> is coming in subsequent patch.
>>>>>
>>>>> for_each_tracepoint_range() api has no users inside the kernel.
>>>>> Make it more useful with ability to stop for_each() loop depending
>>>>> via callback return value.
>>>>> In such form it's used in subsequent patch.
>>>>
>>>> I believe this is used by LTTng.
>>>
>>> Indeed, and by SystemTAP as well.
>>>
>>> What justifies the need to stop mid-iteration ? A less intrusive alternative
>>> would be to use the "priv" data pointer to keep state telling further calls
>>> to return immediately. Does performance of iteration over tracepoints really
>>> matter here so much that stopping iteration immediately is worth it ?
>>
>> I'm sure both you and Steven are not serious when you object
>> to _in-tree_ change to for_each_kernel_tracepoint() that
>> affects _out-of_tree_ modules?
>>
>> Just change your module to 'return NULL' instead of plain 'return'.
>
> I never said I objected to adapt the LTTng out of tree code. If there is a
> solid reason for changing the kernel API, I will adapt my code to those
> changes.
>
> What I'm trying to understand here is whether there is solid ground for
> the added complexity you are proposing. Is it a performance enhancement ?
> If so, explanation of the use cases targeted, and numbers that measure
> performance improvements are needed.
>
> How is your patch making tracepoints "more useful" ?
are you arguing about the whole set overall or about a change
to for_each_kernel_tracepoint() ?
I'm hearing your arguments as that now changes to all exported functions
need to be "solid" (not sure what exactly means 'solid' here) to
justify breakage of out-of-tree modules?
re: 'added complexity'...
- for (iter = begin; iter < end; iter++)
- fct(*iter, priv);
+ return NULL;
+ for (iter = begin; iter < end; iter++) {
+ ret = fct(*iter, priv);
+ if (ret)
+ return ret;
+ }
+ return NULL;
where do you see 'added complexity' ?
Isn't the above diff self-explanatory that for_each_tracepoint_range()
can be used not only to iterate over all tracepoints
(just do 'return NULL') from callback _and_ to find one particular
tracepoint as patch 7 does ?
^ permalink raw reply
* Re: [PATCH] net: qualcomm: rmnet: check for null ep to avoid null pointer dereference
From: David Miller @ 2018-03-26 16:11 UTC (permalink / raw)
To: colin.king; +Cc: subashab, netdev, kernel-janitors, linux-kernel
In-Reply-To: <20180323235157.8129-1-colin.king@canonical.com>
From: Colin King <colin.king@canonical.com>
Date: Fri, 23 Mar 2018 23:51:57 +0000
> From: Colin Ian King <colin.king@canonical.com>
>
> The call to rmnet_get_endpoint can potentially return NULL so check
> for this to avoid any subsequent null pointer dereferences on a NULL
> ep.
>
> Detected by CoverityScan, CID#1465385 ("Dereference null return value")
>
> Fixes: 23790ef12082 ("net: qualcomm: rmnet: Allow to configure flags for existing devices")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Applied to net-next, thanks.
^ permalink raw reply
* Re: [PATCH net-next 0/5] fix some bugs for HNS3
From: David Miller @ 2018-03-26 16:13 UTC (permalink / raw)
To: lipeng321; +Cc: netdev, linux-kernel, linuxarm, salil.mehta
In-Reply-To: <1521862367-111399-1-git-send-email-lipeng321@huawei.com>
From: Peng Li <lipeng321@huawei.com>
Date: Sat, 24 Mar 2018 11:32:42 +0800
> This patchset fixes some bugs for HNS3 driver:
> [Patch 1/5 - 2/5] fix 2 return vlaue issues.
> [Patch 3/5 - 4/5] fix 2 comments reported by code review.
> [Ptach 5/5] avoid sending message to IMP because IMP will not
> handle any message when it is resetting.
Series applied, thank you.
^ permalink raw reply
* Re: [PATCH net-next] ptp: Fix documentation to match code.
From: David Miller @ 2018-03-26 16:13 UTC (permalink / raw)
To: richardcochran; +Cc: netdev
In-Reply-To: <20180324042402.9705-1-richardcochran@gmail.com>
From: Richard Cochran <richardcochran@gmail.com>
Date: Fri, 23 Mar 2018 21:24:02 -0700
> Ever since commit 3a06c7ac24f9 ("posix-clocks: Remove interval timer
> facility and mmap/fasync callbacks") the possibility of PHC based
> posix timers has been removed. In addition it will probably never
> make sense to implement this functionality.
>
> This patch removes the misleading text which seems to suggest that
> posix timers for PHC devices will ever be a thing.
>
> Signed-off-by: Richard Cochran <richardcochran@gmail.com>
Applied.
^ 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