* Re: [PATCH net-next] net: sched: fw: don't set arg->stop in fw_walk() when empty
From: David Miller @ 2019-02-17 21:25 UTC (permalink / raw)
To: vladbu; +Cc: netdev, idosch, jhs, xiyou.wangcong, jiri
In-Reply-To: <20190215152007.8854-1-vladbu@mellanox.com>
From: Vlad Buslov <vladbu@mellanox.com>
Date: Fri, 15 Feb 2019 17:20:07 +0200
> Some classifiers set arg->stop in their implementation of tp->walk() API
> when empty. Most of classifiers do not adhere to that convention. Do not
> set arg->stop in fw_walk() to unify tp->walk() behavior among classifier
> implementations.
>
> Fixes: ed76f5edccc9 ("net: sched: protect filter_chain list with filter_chain_lock mutex")
> Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
Applied.
^ permalink raw reply
* [PATCH] orinoco : Replace function name in string with __func__
From: Keyur Patel @ 2019-02-17 16:25 UTC (permalink / raw)
Cc: iamkeyur96, Kalle Valo, David S. Miller, Eric Biggers, Herbert Xu,
linux-wireless, netdev, linux-kernel
From: Keyur Patel <iamkeyur96@gmail.com>
Replace hard coded function name with __func__, to
improve robustness and to conform to the Linux kernel coding
style. Issue found using checkpatch.
Signed-off-by: Keyur Patel <iamkeyur96@gmail.com>
---
drivers/net/wireless/intersil/orinoco/mic.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wireless/intersil/orinoco/mic.c b/drivers/net/wireless/intersil/orinoco/mic.c
index 709d9ab3e7bc..67b0c05afbdb 100644
--- a/drivers/net/wireless/intersil/orinoco/mic.c
+++ b/drivers/net/wireless/intersil/orinoco/mic.c
@@ -18,16 +18,16 @@ int orinoco_mic_init(struct orinoco_private *priv)
{
priv->tx_tfm_mic = crypto_alloc_shash("michael_mic", 0, 0);
if (IS_ERR(priv->tx_tfm_mic)) {
- printk(KERN_DEBUG "orinoco_mic_init: could not allocate "
- "crypto API michael_mic\n");
+ printk(KERN_DEBUG "%s: could not allocate "
+ "crypto API michael_mic\n", __func__);
priv->tx_tfm_mic = NULL;
return -ENOMEM;
}
priv->rx_tfm_mic = crypto_alloc_shash("michael_mic", 0, 0);
if (IS_ERR(priv->rx_tfm_mic)) {
- printk(KERN_DEBUG "orinoco_mic_init: could not allocate "
- "crypto API michael_mic\n");
+ printk(KERN_DEBUG "%s: could not allocate "
+ "crypto API michael_mic\n", __func__);
priv->rx_tfm_mic = NULL;
return -ENOMEM;
}
@@ -52,7 +52,7 @@ int orinoco_mic(struct crypto_shash *tfm_michael, u8 *key,
int err;
if (tfm_michael == NULL) {
- printk(KERN_WARNING "orinoco_mic: tfm_michael == NULL\n");
+ printk(KERN_WARNING "%s: tfm_michael == NULL\n", __func__);
return -1;
}
--
2.20.1
^ permalink raw reply related
* Re: [PATCH] net: sched: matchall: verify that filter is not NULL in mall_walk()
From: David Miller @ 2019-02-17 21:27 UTC (permalink / raw)
To: vladbu; +Cc: idosch, netdev, jhs, xiyou.wangcong, jiri
In-Reply-To: <20190215121120.4971-1-vladbu@mellanox.com>
From: Vlad Buslov <vladbu@mellanox.com>
Date: Fri, 15 Feb 2019 14:11:20 +0200
> Check that filter is not NULL before passing it to tcf_walker->fn()
> callback. This can happen when mall_change() failed to offload filter to
> hardware.
>
> Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next 1/3] net: dsa: add support for bridge flags
From: Florian Fainelli @ 2019-02-17 21:37 UTC (permalink / raw)
To: Russell King, Andrew Lunn, Vivien Didelot; +Cc: David S. Miller, netdev
In-Reply-To: <E1gvNNT-0006Xr-JA@rmk-PC.armlinux.org.uk>
On 2/17/2019 6:25 AM, Russell King wrote:
> The Linux bridge implementation allows various properties of the bridge
> to be controlled, such as flooding unknown unicast and multicast frames.
> This patch adds the necessary DSA infrastructure to allow the Linux
> bridge support to control these properties for DSA switches.
>
> We implement this by providing two new methods: one to get the switch-
> wide support bitmask, and another to set the properties.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
[snip]
>
> +int dsa_port_bridge_flags(const struct dsa_port *dp, unsigned long flags,
> + struct switchdev_trans *trans)
> +{
> + struct dsa_switch *ds = dp->ds;
> + int port = dp->index;
> +
> + if (switchdev_trans_ph_prepare(trans))
> + return ds->ops->port_bridge_flags ? 0 : -EOPNOTSUPP;
> +
> + if (ds->ops->port_bridge_flags)
> + ds->ops->port_bridge_flags(ds, port, flags);
If you have a switch fabric with multiple switches, it seems to me that
you also need to make sure that the DSA and CPU ports will have
compatible flooding attribute, so just like the port_vlan_add()
callback, you probably need to make this a switch fabric-wide event and
use a notifier here. At least the DSA ports need to have MC flooding
turned on for an user port to also have MC flooding working.
Other than that LGTM.
--
Florian
^ permalink raw reply
* Re: [PATCH net-next 2/3] net: dsa: mv88e6xxx: add support for bridge flags
From: Florian Fainelli @ 2019-02-17 21:38 UTC (permalink / raw)
To: Russell King, Andrew Lunn, Vivien Didelot; +Cc: David S. Miller, netdev
In-Reply-To: <E1gvNNY-0006Y7-NF@rmk-PC.armlinux.org.uk>
On 2/17/2019 6:25 AM, Russell King wrote:
> Add support for the bridge flags to Marvell 88e6xxx bridges, allowing
> the multicast and unicast flood properties to be controlled. These
> can be controlled on a per-port basis via commands such as:
>
> bridge link set dev lan1 flood on|off
> bridge link set dev lan1 mcast_flood on|off
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH net-next 3/3] net: dsa: mv88e6xxx: defautl to multicast and unicast flooding
From: Florian Fainelli @ 2019-02-17 21:45 UTC (permalink / raw)
To: Russell King - ARM Linux admin, Andrew Lunn, Vivien Didelot
Cc: David S. Miller, netdev
In-Reply-To: <20190217163434.xo3ojbny4dnfqqwq@shell.armlinux.org.uk>
On 2/17/2019 8:34 AM, Russell King - ARM Linux admin wrote:
> On Sun, Feb 17, 2019 at 02:27:16PM +0000, Russell King - ARM Linux admin wrote:
>> On Sun, Feb 17, 2019 at 02:25:17PM +0000, Russell King wrote:
>>> Switches work by learning the MAC address for each attached station by
>>> monitoring traffic from each station. When a station sends a packet,
>>> the switch records which port the MAC address is connected to.
>>>
>>> With IPv4 networking, before communication commences with a neighbour,
>>> an ARP packet is broadcasted to all stations asking for the MAC address
>>> corresponding with the IPv4. The desired station responds with an ARP
>>> reply, and the ARP reply causes the switch to learn which port the
>>> station is connected to.
>>>
>>> With IPv6 networking, the situation is rather different. Rather than
>>> broadcasting ARP packets, a "neighbour solicitation" is multicasted
>>> rather than broadcasted. This multicast needs to reach the intended
>>> station in order for the neighbour to be discovered.
>>>
>>> Once a neighbour has been discovered, and entered into the sending
>>> stations neighbour cache, communication can restart at a point later
>>> without sending a new neighbour solicitation, even if the entry in
>>> the neighbour cache is marked as stale. This can be after the MAC
>>> address has expired from the forwarding cache of the DSA switch -
>>> when that occurs, there is a long pause in communication.
>>>
>>> Our DSA implementation for mv88e6xxx switches has defaulted to having
>>> multicast and unicast flooding disabled. As per the above description,
>>> this is fine for IPv4 networking, since the broadcasted ARP queries
>>> will be sent to and received by all stations on the same network.
>>> However, this breaks IPv6 very badly - blocking neighbour solicitations
>>> and later causing connections to stall.
>>>
>>> The defaults that the Linux bridge code expect from bridges are that
>>> unknown unicast frames and unknown multicast frames are flooded to
>>> all stations, which is at odds to the defaults adopted by our DSA
>>> implementation for mv88e6xxx switches.
>>>
>>> This commit enables by default flooding of both unknown unicast and
>>> unknown multicast frames. This means that mv88e6xxx DSA switches now
>>> behave as per the bridge(8) man page, and IPv6 works flawlessly through
>>> such a switch.
>>
>> Note that there is the open question whether this affects the case where
>> each port is used as a separate network interface: that case has not yet
>> been tested.
>
> I've checked with a mv88e6131 on the clearfog gt8k board. lan1
> connected to my lan with plenty of traffic on, and configured as
> part of a bridge. lan2 connected to the zii board, but not part
> of the bridge. Monitoring lan2 from the zii board shows no traffic
> that was received from lan1.
>
> So it looks fine.
With the current state whereby we do not have the necessary hooks to
perform filtering on non-bridged/standalone ports, this is entirely fine
indeed.
In the future this is part of something I want to address because it is
IMHO highly undesirable to have non-bridged ports be flooded with
unknown multicast or unknown unicast for that matter because that makes
them deviate from a standard NIC interface. Unknown unicast is not
necessarily a low hanging fruit, but still, if we have switches capable
of filtering, we might as well make use of that. Of course, one
difficulty is that we must not break running tcpdump on those DSA slave
network interfaces.
>
>>
>>>
>>> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
>>> ---
>>> drivers/net/dsa/mv88e6xxx/chip.c | 9 +++++----
>>> 1 file changed, 5 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
>>> index b75a865a293d..eb5e3d88374f 100644
>>> --- a/drivers/net/dsa/mv88e6xxx/chip.c
>>> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
>>> @@ -2144,13 +2144,14 @@ static int mv88e6xxx_setup_message_port(struct mv88e6xxx_chip *chip, int port)
>>> static int mv88e6xxx_setup_egress_floods(struct mv88e6xxx_chip *chip, int port)
>>> {
>>> struct dsa_switch *ds = chip->ds;
>>> - bool flood;
>>>
>>> - /* Upstream ports flood frames with unknown unicast or multicast DA */
>>> - flood = dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port);
>>> + /* Linux bridges are expected to flood unknown multicast and
>>> + * unicast frames to all ports - as per the defaults specified
>>> + * in the iproute2 bridge(8) man page. Not doing this causes
>>> + * stalls and failures with IPv6 over Marvell bridges. */
>>> if (chip->info->ops->port_set_egress_floods)
>>> return chip->info->ops->port_set_egress_floods(chip, port,
>>> - flood, flood);
>>> + true, true);
>>>
>>> return 0;
>>> }
>>> --
>>> 2.7.4
>>>
>>>
>>
>> --
>> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
>> FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
>> According to speedtest.net: 11.9Mbps down 500kbps up
>
--
Florian
^ permalink raw reply
* Re: [PATCH v3 perf,bpf 00/11] perf annotation of BPF programs
From: Jiri Olsa @ 2019-02-17 21:57 UTC (permalink / raw)
To: Song Liu
Cc: netdev, linux-kernel, ast, daniel, kernel-team, peterz, acme,
jolsa, namhyung
In-Reply-To: <20190215215354.3114006-1-songliubraving@fb.com>
On Fri, Feb 15, 2019 at 01:53:43PM -0800, Song Liu wrote:
> Changes v2 to v3:
> 1. Remove unnecessary include in header files;
> 2. Improved error handling;
> 3. Better naming of functions, variables, etc.;
> 4. Enable bpf events by default for perf-top.
>
> Changes v1 to v2:
> 1. Fix compilation error with different feature-disassembler-four-args;
> 2. Fix a segfault in perf-record;
> 3. Split patches 5/9 and 6/9 so that perf_env changes and perf.data changes
> are in separate patches.
>
> This series enables annotation of BPF programs in perf.
>
> perf tool gathers information via sys_bpf and (optionally) stores them in
> perf.data as headers.
>
> Patch 1/11 fixes a minor issue in kernel;
> Patch 2/11 to 4/11 introduce new helper functions and use them in perf and
> bpftool;
> Patch 5/11 to 8/11 saves information of bpf program in perf_env;
> Patch 9/11 adds --bpf-event options to perf-top;
> Patch 10/11 enables annotation of bpf programs based on information
> gathered in 5/11 to 8/11;
> Patch 11/11 handles information of short living BPF program that are loaded
> during perf-record or perf-top.
>
> Commands tested during developments are perf-top, perf-record, perf-report,
> and perf-annotate.
>
> ===================== Note on patch dependency ========================
> This set has dependency in both bpf-next tree and tip/perf/core. Current
> version is developed on bpf-next tree with the following commits
> cherry-picked from tip/perf/core:
>
> (from 1/10 to 10/10)
> commit 76193a94522f ("perf, bpf: Introduce PERF_RECORD_KSYMBOL")
> commit d764ac646491 ("tools headers uapi: Sync tools/include/uapi/linux/perf_event.h")
> commit 6ee52e2a3fe4 ("perf, bpf: Introduce PERF_RECORD_BPF_EVENT")
> commit df063c83aa2c ("tools headers uapi: Sync tools/include/uapi/linux/perf_event.h")
> commit 9aa0bfa370b2 ("perf tools: Handle PERF_RECORD_KSYMBOL")
> commit 45178a928a4b ("perf tools: Handle PERF_RECORD_BPF_EVENT")
> commit 7b612e291a5a ("perf tools: Synthesize PERF_RECORD_* for loaded BPF programs")
> commit a40b95bcd30c ("perf top: Synthesize BPF events for pre-existing loaded BPF programs")
> commit 6934058d9fb6 ("bpf: Add module name [bpf] to ksymbols for bpf programs")
> commit 811184fb6977 ("perf bpf: Fix synthesized PERF_RECORD_KSYMBOL/BPF_EVENT")
> ========================================================================
>
> This set is also available at:
>
> https://github.com/liu-song-6/linux/tree/bpf-annotation
I'm getting same compilation error as last time,
maybe it wasnt updated?
thanks,
jirka
^ permalink raw reply
* Re: [PATCH net-next 3/3] net: dsa: mv88e6xxx: defautl to multicast and unicast flooding
From: Russell King - ARM Linux admin @ 2019-02-17 21:58 UTC (permalink / raw)
To: Florian Fainelli; +Cc: Andrew Lunn, Vivien Didelot, David S. Miller, netdev
In-Reply-To: <1a956aeb-f195-90ca-a5a5-fabac450feac@gmail.com>
On Sun, Feb 17, 2019 at 01:45:24PM -0800, Florian Fainelli wrote:
>
>
> On 2/17/2019 8:34 AM, Russell King - ARM Linux admin wrote:
> > On Sun, Feb 17, 2019 at 02:27:16PM +0000, Russell King - ARM Linux admin wrote:
> >> On Sun, Feb 17, 2019 at 02:25:17PM +0000, Russell King wrote:
> >>> Switches work by learning the MAC address for each attached station by
> >>> monitoring traffic from each station. When a station sends a packet,
> >>> the switch records which port the MAC address is connected to.
> >>>
> >>> With IPv4 networking, before communication commences with a neighbour,
> >>> an ARP packet is broadcasted to all stations asking for the MAC address
> >>> corresponding with the IPv4. The desired station responds with an ARP
> >>> reply, and the ARP reply causes the switch to learn which port the
> >>> station is connected to.
> >>>
> >>> With IPv6 networking, the situation is rather different. Rather than
> >>> broadcasting ARP packets, a "neighbour solicitation" is multicasted
> >>> rather than broadcasted. This multicast needs to reach the intended
> >>> station in order for the neighbour to be discovered.
> >>>
> >>> Once a neighbour has been discovered, and entered into the sending
> >>> stations neighbour cache, communication can restart at a point later
> >>> without sending a new neighbour solicitation, even if the entry in
> >>> the neighbour cache is marked as stale. This can be after the MAC
> >>> address has expired from the forwarding cache of the DSA switch -
> >>> when that occurs, there is a long pause in communication.
> >>>
> >>> Our DSA implementation for mv88e6xxx switches has defaulted to having
> >>> multicast and unicast flooding disabled. As per the above description,
> >>> this is fine for IPv4 networking, since the broadcasted ARP queries
> >>> will be sent to and received by all stations on the same network.
> >>> However, this breaks IPv6 very badly - blocking neighbour solicitations
> >>> and later causing connections to stall.
> >>>
> >>> The defaults that the Linux bridge code expect from bridges are that
> >>> unknown unicast frames and unknown multicast frames are flooded to
> >>> all stations, which is at odds to the defaults adopted by our DSA
> >>> implementation for mv88e6xxx switches.
> >>>
> >>> This commit enables by default flooding of both unknown unicast and
> >>> unknown multicast frames. This means that mv88e6xxx DSA switches now
> >>> behave as per the bridge(8) man page, and IPv6 works flawlessly through
> >>> such a switch.
> >>
> >> Note that there is the open question whether this affects the case where
> >> each port is used as a separate network interface: that case has not yet
> >> been tested.
> >
> > I've checked with a mv88e6131 on the clearfog gt8k board. lan1
> > connected to my lan with plenty of traffic on, and configured as
> > part of a bridge. lan2 connected to the zii board, but not part
> > of the bridge. Monitoring lan2 from the zii board shows no traffic
> > that was received from lan1.
> >
> > So it looks fine.
>
> With the current state whereby we do not have the necessary hooks to
> perform filtering on non-bridged/standalone ports, this is entirely fine
> indeed.
>
> In the future this is part of something I want to address because it is
> IMHO highly undesirable to have non-bridged ports be flooded with
> unknown multicast or unknown unicast for that matter because that makes
> them deviate from a standard NIC interface. Unknown unicast is not
> necessarily a low hanging fruit, but still, if we have switches capable
> of filtering, we might as well make use of that. Of course, one
> difficulty is that we must not break running tcpdump on those DSA slave
> network interfaces.
Sorry, I think you have the wrong end of the stick.
For a non-bridged port, I am seeing _no_ traffic apart from that
explicitly sent out through that port. In other words, there are
_no_ flooded frames coming out of the non-bridged port.
This patch appears to have no material effect on non-bridged ports.
>
> >
> >>
> >>>
> >>> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> >>> ---
> >>> drivers/net/dsa/mv88e6xxx/chip.c | 9 +++++----
> >>> 1 file changed, 5 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> >>> index b75a865a293d..eb5e3d88374f 100644
> >>> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> >>> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> >>> @@ -2144,13 +2144,14 @@ static int mv88e6xxx_setup_message_port(struct mv88e6xxx_chip *chip, int port)
> >>> static int mv88e6xxx_setup_egress_floods(struct mv88e6xxx_chip *chip, int port)
> >>> {
> >>> struct dsa_switch *ds = chip->ds;
> >>> - bool flood;
> >>>
> >>> - /* Upstream ports flood frames with unknown unicast or multicast DA */
> >>> - flood = dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port);
> >>> + /* Linux bridges are expected to flood unknown multicast and
> >>> + * unicast frames to all ports - as per the defaults specified
> >>> + * in the iproute2 bridge(8) man page. Not doing this causes
> >>> + * stalls and failures with IPv6 over Marvell bridges. */
> >>> if (chip->info->ops->port_set_egress_floods)
> >>> return chip->info->ops->port_set_egress_floods(chip, port,
> >>> - flood, flood);
> >>> + true, true);
> >>>
> >>> return 0;
> >>> }
> >>> --
> >>> 2.7.4
> >>>
> >>>
> >>
> >> --
> >> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> >> FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
> >> According to speedtest.net: 11.9Mbps down 500kbps up
> >
>
> --
> Florian
>
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
^ permalink raw reply
* Re: [PATCH net-next 3/3] net: dsa: mv88e6xxx: defautl to multicast and unicast flooding
From: Florian Fainelli @ 2019-02-17 22:03 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: Andrew Lunn, Vivien Didelot, David S. Miller, netdev
In-Reply-To: <20190217215827.2iavpy4xyh3viqup@shell.armlinux.org.uk>
On 2/17/2019 1:58 PM, Russell King - ARM Linux admin wrote:
> On Sun, Feb 17, 2019 at 01:45:24PM -0800, Florian Fainelli wrote:
>>
>>
>> On 2/17/2019 8:34 AM, Russell King - ARM Linux admin wrote:
>>> On Sun, Feb 17, 2019 at 02:27:16PM +0000, Russell King - ARM Linux admin wrote:
>>>> On Sun, Feb 17, 2019 at 02:25:17PM +0000, Russell King wrote:
>>>>> Switches work by learning the MAC address for each attached station by
>>>>> monitoring traffic from each station. When a station sends a packet,
>>>>> the switch records which port the MAC address is connected to.
>>>>>
>>>>> With IPv4 networking, before communication commences with a neighbour,
>>>>> an ARP packet is broadcasted to all stations asking for the MAC address
>>>>> corresponding with the IPv4. The desired station responds with an ARP
>>>>> reply, and the ARP reply causes the switch to learn which port the
>>>>> station is connected to.
>>>>>
>>>>> With IPv6 networking, the situation is rather different. Rather than
>>>>> broadcasting ARP packets, a "neighbour solicitation" is multicasted
>>>>> rather than broadcasted. This multicast needs to reach the intended
>>>>> station in order for the neighbour to be discovered.
>>>>>
>>>>> Once a neighbour has been discovered, and entered into the sending
>>>>> stations neighbour cache, communication can restart at a point later
>>>>> without sending a new neighbour solicitation, even if the entry in
>>>>> the neighbour cache is marked as stale. This can be after the MAC
>>>>> address has expired from the forwarding cache of the DSA switch -
>>>>> when that occurs, there is a long pause in communication.
>>>>>
>>>>> Our DSA implementation for mv88e6xxx switches has defaulted to having
>>>>> multicast and unicast flooding disabled. As per the above description,
>>>>> this is fine for IPv4 networking, since the broadcasted ARP queries
>>>>> will be sent to and received by all stations on the same network.
>>>>> However, this breaks IPv6 very badly - blocking neighbour solicitations
>>>>> and later causing connections to stall.
>>>>>
>>>>> The defaults that the Linux bridge code expect from bridges are that
>>>>> unknown unicast frames and unknown multicast frames are flooded to
>>>>> all stations, which is at odds to the defaults adopted by our DSA
>>>>> implementation for mv88e6xxx switches.
>>>>>
>>>>> This commit enables by default flooding of both unknown unicast and
>>>>> unknown multicast frames. This means that mv88e6xxx DSA switches now
>>>>> behave as per the bridge(8) man page, and IPv6 works flawlessly through
>>>>> such a switch.
>>>>
>>>> Note that there is the open question whether this affects the case where
>>>> each port is used as a separate network interface: that case has not yet
>>>> been tested.
>>>
>>> I've checked with a mv88e6131 on the clearfog gt8k board. lan1
>>> connected to my lan with plenty of traffic on, and configured as
>>> part of a bridge. lan2 connected to the zii board, but not part
>>> of the bridge. Monitoring lan2 from the zii board shows no traffic
>>> that was received from lan1.
>>>
>>> So it looks fine.
>>
>> With the current state whereby we do not have the necessary hooks to
>> perform filtering on non-bridged/standalone ports, this is entirely fine
>> indeed.
>>
>> In the future this is part of something I want to address because it is
>> IMHO highly undesirable to have non-bridged ports be flooded with
>> unknown multicast or unknown unicast for that matter because that makes
>> them deviate from a standard NIC interface. Unknown unicast is not
>> necessarily a low hanging fruit, but still, if we have switches capable
>> of filtering, we might as well make use of that. Of course, one
>> difficulty is that we must not break running tcpdump on those DSA slave
>> network interfaces.
>
> Sorry, I think you have the wrong end of the stick.
>
> For a non-bridged port, I am seeing _no_ traffic apart from that
> explicitly sent out through that port. In other words, there are
> _no_ flooded frames coming out of the non-bridged port.
>
> This patch appears to have no material effect on non-bridged ports.
Presumably because that non-bridged port and the CPU port are part of
the same domain with only those 2 ports and that is what we want.
Now what happens if say you have a station that sends multicast traffic
through that port to e.g.: 226.94.1.1, I bet that port happily sends
that multicast traffic to the CPU port with no filtering what so ever
and this ends-up being dropped in the network stack because there is a
socket look up failure there. IMHO unless you have a receiver/server on
that network interface on the DSA network interface and a matching
socket you should not be receiving that multicast traffic and the switch
should be filtering it. Since the network stack will call into
ndo_set_rx_mode() for those cases, we really just need to make that
multicast traffic known, instead of unknown to the switch.
--
Florian
^ permalink raw reply
* Re: [PATCH v3 perf,bpf 00/11] perf annotation of BPF programs
From: Jiri Olsa @ 2019-02-17 22:04 UTC (permalink / raw)
To: Song Liu
Cc: netdev, linux-kernel, ast, daniel, kernel-team, peterz, acme,
jolsa, namhyung
In-Reply-To: <20190217215744.GA797@krava>
On Sun, Feb 17, 2019 at 10:57:44PM +0100, Jiri Olsa wrote:
> On Fri, Feb 15, 2019 at 01:53:43PM -0800, Song Liu wrote:
> > Changes v2 to v3:
> > 1. Remove unnecessary include in header files;
> > 2. Improved error handling;
> > 3. Better naming of functions, variables, etc.;
> > 4. Enable bpf events by default for perf-top.
> >
> > Changes v1 to v2:
> > 1. Fix compilation error with different feature-disassembler-four-args;
> > 2. Fix a segfault in perf-record;
> > 3. Split patches 5/9 and 6/9 so that perf_env changes and perf.data changes
> > are in separate patches.
> >
> > This series enables annotation of BPF programs in perf.
> >
> > perf tool gathers information via sys_bpf and (optionally) stores them in
> > perf.data as headers.
> >
> > Patch 1/11 fixes a minor issue in kernel;
> > Patch 2/11 to 4/11 introduce new helper functions and use them in perf and
> > bpftool;
> > Patch 5/11 to 8/11 saves information of bpf program in perf_env;
> > Patch 9/11 adds --bpf-event options to perf-top;
> > Patch 10/11 enables annotation of bpf programs based on information
> > gathered in 5/11 to 8/11;
> > Patch 11/11 handles information of short living BPF program that are loaded
> > during perf-record or perf-top.
> >
> > Commands tested during developments are perf-top, perf-record, perf-report,
> > and perf-annotate.
> >
> > ===================== Note on patch dependency ========================
> > This set has dependency in both bpf-next tree and tip/perf/core. Current
> > version is developed on bpf-next tree with the following commits
> > cherry-picked from tip/perf/core:
> >
> > (from 1/10 to 10/10)
> > commit 76193a94522f ("perf, bpf: Introduce PERF_RECORD_KSYMBOL")
> > commit d764ac646491 ("tools headers uapi: Sync tools/include/uapi/linux/perf_event.h")
> > commit 6ee52e2a3fe4 ("perf, bpf: Introduce PERF_RECORD_BPF_EVENT")
> > commit df063c83aa2c ("tools headers uapi: Sync tools/include/uapi/linux/perf_event.h")
> > commit 9aa0bfa370b2 ("perf tools: Handle PERF_RECORD_KSYMBOL")
> > commit 45178a928a4b ("perf tools: Handle PERF_RECORD_BPF_EVENT")
> > commit 7b612e291a5a ("perf tools: Synthesize PERF_RECORD_* for loaded BPF programs")
> > commit a40b95bcd30c ("perf top: Synthesize BPF events for pre-existing loaded BPF programs")
> > commit 6934058d9fb6 ("bpf: Add module name [bpf] to ksymbols for bpf programs")
> > commit 811184fb6977 ("perf bpf: Fix synthesized PERF_RECORD_KSYMBOL/BPF_EVENT")
> > ========================================================================
> >
> > This set is also available at:
> >
> > https://github.com/liu-song-6/linux/tree/bpf-annotation
>
> I'm getting same compilation error as last time,
> maybe it wasnt updated?
I also applied manualy (with some conflicts) your patches
from emails on bpf/next/master with merged tip/perf/core
and got following compilation error:
CC util/find_bit.o
util/annotate.c: In function ‘symbol__disassemble_bpf’:
util/annotate.c:1771:29: error: incompatible type for argument 1 of ‘disassembler’
disassemble = disassembler(bfdf);
^~~~
In file included from util/annotate.c:36:
/usr/include/dis-asm.h:276:63: note: expected ‘enum bfd_architecture’ but argument is of type ‘bfd *’ {aka ‘struct bfd *’}
extern disassembler_ftype disassembler (enum bfd_architecture arc,
~~~~~~~~~~~~~~~~~~~~~~^~~
util/annotate.c:1771:16: error: too few arguments to function ‘disassembler’
disassemble = disassembler(bfdf);
^~~~~~~~~~~~
In file included from util/annotate.c:36:
/usr/include/dis-asm.h:276:27: note: declared here
extern disassembler_ftype disassembler (enum bfd_architecture arc,
^~~~~~~~~~~~
thanks,
jirka
^ permalink raw reply
* Re: [PATCH net-next 1/3] net: dsa: add support for bridge flags
From: Russell King - ARM Linux admin @ 2019-02-17 22:04 UTC (permalink / raw)
To: Florian Fainelli; +Cc: Andrew Lunn, Vivien Didelot, David S. Miller, netdev
In-Reply-To: <a3c3a343-a49d-a93a-929a-9a1b1304129e@gmail.com>
On Sun, Feb 17, 2019 at 01:37:19PM -0800, Florian Fainelli wrote:
>
>
> On 2/17/2019 6:25 AM, Russell King wrote:
> > The Linux bridge implementation allows various properties of the bridge
> > to be controlled, such as flooding unknown unicast and multicast frames.
> > This patch adds the necessary DSA infrastructure to allow the Linux
> > bridge support to control these properties for DSA switches.
> >
> > We implement this by providing two new methods: one to get the switch-
> > wide support bitmask, and another to set the properties.
> >
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > ---
>
> [snip]
>
> >
> > +int dsa_port_bridge_flags(const struct dsa_port *dp, unsigned long flags,
> > + struct switchdev_trans *trans)
> > +{
> > + struct dsa_switch *ds = dp->ds;
> > + int port = dp->index;
> > +
> > + if (switchdev_trans_ph_prepare(trans))
> > + return ds->ops->port_bridge_flags ? 0 : -EOPNOTSUPP;
> > +
> > + if (ds->ops->port_bridge_flags)
> > + ds->ops->port_bridge_flags(ds, port, flags);
>
> If you have a switch fabric with multiple switches, it seems to me that
> you also need to make sure that the DSA and CPU ports will have
> compatible flooding attribute, so just like the port_vlan_add()
> callback, you probably need to make this a switch fabric-wide event and
> use a notifier here. At least the DSA ports need to have MC flooding
> turned on for an user port to also have MC flooding working.
mv88e6xxx already today detects CPU and DSA ports and sets unicast
and multicast flooding for these ports - see
mv88e6xxx_setup_egress_floods():
/* Upstream ports flood frames with unknown unicast or multicast DA */
flood = dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port);
if (chip->info->ops->port_set_egress_floods)
return chip->info->ops->port_set_egress_floods(chip, port,
flood, flood);
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
^ permalink raw reply
* Re: [PATCH net-next 1/3] net: dsa: add support for bridge flags
From: Florian Fainelli @ 2019-02-17 22:07 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: Andrew Lunn, Vivien Didelot, David S. Miller, netdev
In-Reply-To: <20190217220439.zqu7dz7kfukssrlz@shell.armlinux.org.uk>
On 2/17/2019 2:04 PM, Russell King - ARM Linux admin wrote:
> On Sun, Feb 17, 2019 at 01:37:19PM -0800, Florian Fainelli wrote:
>>
>>
>> On 2/17/2019 6:25 AM, Russell King wrote:
>>> The Linux bridge implementation allows various properties of the bridge
>>> to be controlled, such as flooding unknown unicast and multicast frames.
>>> This patch adds the necessary DSA infrastructure to allow the Linux
>>> bridge support to control these properties for DSA switches.
>>>
>>> We implement this by providing two new methods: one to get the switch-
>>> wide support bitmask, and another to set the properties.
>>>
>>> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
>>> ---
>>
>> [snip]
>>
>>>
>>> +int dsa_port_bridge_flags(const struct dsa_port *dp, unsigned long flags,
>>> + struct switchdev_trans *trans)
>>> +{
>>> + struct dsa_switch *ds = dp->ds;
>>> + int port = dp->index;
>>> +
>>> + if (switchdev_trans_ph_prepare(trans))
>>> + return ds->ops->port_bridge_flags ? 0 : -EOPNOTSUPP;
>>> +
>>> + if (ds->ops->port_bridge_flags)
>>> + ds->ops->port_bridge_flags(ds, port, flags);
>>
>> If you have a switch fabric with multiple switches, it seems to me that
>> you also need to make sure that the DSA and CPU ports will have
>> compatible flooding attribute, so just like the port_vlan_add()
>> callback, you probably need to make this a switch fabric-wide event and
>> use a notifier here. At least the DSA ports need to have MC flooding
>> turned on for an user port to also have MC flooding working.
>
> mv88e6xxx already today detects CPU and DSA ports and sets unicast
> and multicast flooding for these ports - see
> mv88e6xxx_setup_egress_floods():
Indeed, probably for historical reasons, since that type of logic should
ideally be migrated to the core DSA layer, this is fine for now though.
>
> /* Upstream ports flood frames with unknown unicast or multicast DA */
> flood = dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port);
> if (chip->info->ops->port_set_egress_floods)
> return chip->info->ops->port_set_egress_floods(chip, port,
> flood, flood);
>
--
Florian
^ permalink raw reply
* Re: [PATCH net-next 3/3] net: dsa: mv88e6xxx: defautl to multicast and unicast flooding
From: Russell King - ARM Linux admin @ 2019-02-17 22:19 UTC (permalink / raw)
To: Florian Fainelli; +Cc: Andrew Lunn, Vivien Didelot, David S. Miller, netdev
In-Reply-To: <39dbb59c-0c67-95f8-6703-d3e628d8c50e@gmail.com>
On Sun, Feb 17, 2019 at 02:03:40PM -0800, Florian Fainelli wrote:
>
>
> On 2/17/2019 1:58 PM, Russell King - ARM Linux admin wrote:
> > On Sun, Feb 17, 2019 at 01:45:24PM -0800, Florian Fainelli wrote:
> >>
> >>
> >> On 2/17/2019 8:34 AM, Russell King - ARM Linux admin wrote:
> >>> On Sun, Feb 17, 2019 at 02:27:16PM +0000, Russell King - ARM Linux admin wrote:
> >>>> On Sun, Feb 17, 2019 at 02:25:17PM +0000, Russell King wrote:
> >>>>> Switches work by learning the MAC address for each attached station by
> >>>>> monitoring traffic from each station. When a station sends a packet,
> >>>>> the switch records which port the MAC address is connected to.
> >>>>>
> >>>>> With IPv4 networking, before communication commences with a neighbour,
> >>>>> an ARP packet is broadcasted to all stations asking for the MAC address
> >>>>> corresponding with the IPv4. The desired station responds with an ARP
> >>>>> reply, and the ARP reply causes the switch to learn which port the
> >>>>> station is connected to.
> >>>>>
> >>>>> With IPv6 networking, the situation is rather different. Rather than
> >>>>> broadcasting ARP packets, a "neighbour solicitation" is multicasted
> >>>>> rather than broadcasted. This multicast needs to reach the intended
> >>>>> station in order for the neighbour to be discovered.
> >>>>>
> >>>>> Once a neighbour has been discovered, and entered into the sending
> >>>>> stations neighbour cache, communication can restart at a point later
> >>>>> without sending a new neighbour solicitation, even if the entry in
> >>>>> the neighbour cache is marked as stale. This can be after the MAC
> >>>>> address has expired from the forwarding cache of the DSA switch -
> >>>>> when that occurs, there is a long pause in communication.
> >>>>>
> >>>>> Our DSA implementation for mv88e6xxx switches has defaulted to having
> >>>>> multicast and unicast flooding disabled. As per the above description,
> >>>>> this is fine for IPv4 networking, since the broadcasted ARP queries
> >>>>> will be sent to and received by all stations on the same network.
> >>>>> However, this breaks IPv6 very badly - blocking neighbour solicitations
> >>>>> and later causing connections to stall.
> >>>>>
> >>>>> The defaults that the Linux bridge code expect from bridges are that
> >>>>> unknown unicast frames and unknown multicast frames are flooded to
> >>>>> all stations, which is at odds to the defaults adopted by our DSA
> >>>>> implementation for mv88e6xxx switches.
> >>>>>
> >>>>> This commit enables by default flooding of both unknown unicast and
> >>>>> unknown multicast frames. This means that mv88e6xxx DSA switches now
> >>>>> behave as per the bridge(8) man page, and IPv6 works flawlessly through
> >>>>> such a switch.
> >>>>
> >>>> Note that there is the open question whether this affects the case where
> >>>> each port is used as a separate network interface: that case has not yet
> >>>> been tested.
> >>>
> >>> I've checked with a mv88e6131 on the clearfog gt8k board. lan1
> >>> connected to my lan with plenty of traffic on, and configured as
> >>> part of a bridge. lan2 connected to the zii board, but not part
> >>> of the bridge. Monitoring lan2 from the zii board shows no traffic
> >>> that was received from lan1.
> >>>
> >>> So it looks fine.
> >>
> >> With the current state whereby we do not have the necessary hooks to
> >> perform filtering on non-bridged/standalone ports, this is entirely fine
> >> indeed.
> >>
> >> In the future this is part of something I want to address because it is
> >> IMHO highly undesirable to have non-bridged ports be flooded with
> >> unknown multicast or unknown unicast for that matter because that makes
> >> them deviate from a standard NIC interface. Unknown unicast is not
> >> necessarily a low hanging fruit, but still, if we have switches capable
> >> of filtering, we might as well make use of that. Of course, one
> >> difficulty is that we must not break running tcpdump on those DSA slave
> >> network interfaces.
> >
> > Sorry, I think you have the wrong end of the stick.
> >
> > For a non-bridged port, I am seeing _no_ traffic apart from that
> > explicitly sent out through that port. In other words, there are
> > _no_ flooded frames coming out of the non-bridged port.
> >
> > This patch appears to have no material effect on non-bridged ports.
>
> Presumably because that non-bridged port and the CPU port are part of
> the same domain with only those 2 ports and that is what we want.
>
> Now what happens if say you have a station that sends multicast traffic
> through that port to e.g.: 226.94.1.1, I bet that port happily sends
> that multicast traffic to the CPU port with no filtering what so ever
> and this ends-up being dropped in the network stack because there is a
> socket look up failure there. IMHO unless you have a receiver/server on
> that network interface on the DSA network interface and a matching
> socket you should not be receiving that multicast traffic and the switch
> should be filtering it. Since the network stack will call into
> ndo_set_rx_mode() for those cases, we really just need to make that
> multicast traffic known, instead of unknown to the switch.
If the port is not bridged, then it's operating as network interface,
and traffic to/from that port needs to be routed to the CPU port so
that it appears as it would do from a real network interface.
Doing anything else makes breaks the idea that you can use a set
of DSA ports as individual interfaces and run anything but IPv4
non-multicast over them.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
^ permalink raw reply
* Re: [PATCH net-next 3/3] net: dsa: mv88e6xxx: defautl to multicast and unicast flooding
From: Florian Fainelli @ 2019-02-17 22:30 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: Andrew Lunn, Vivien Didelot, David S. Miller, netdev
In-Reply-To: <20190217221927.5efgzd5buy6e4sg3@shell.armlinux.org.uk>
On 2/17/2019 2:19 PM, Russell King - ARM Linux admin wrote:
> On Sun, Feb 17, 2019 at 02:03:40PM -0800, Florian Fainelli wrote:
>>
>>
>> On 2/17/2019 1:58 PM, Russell King - ARM Linux admin wrote:
>>> On Sun, Feb 17, 2019 at 01:45:24PM -0800, Florian Fainelli wrote:
>>>>
>>>>
>>>> On 2/17/2019 8:34 AM, Russell King - ARM Linux admin wrote:
>>>>> On Sun, Feb 17, 2019 at 02:27:16PM +0000, Russell King - ARM Linux admin wrote:
>>>>>> On Sun, Feb 17, 2019 at 02:25:17PM +0000, Russell King wrote:
>>>>>>> Switches work by learning the MAC address for each attached station by
>>>>>>> monitoring traffic from each station. When a station sends a packet,
>>>>>>> the switch records which port the MAC address is connected to.
>>>>>>>
>>>>>>> With IPv4 networking, before communication commences with a neighbour,
>>>>>>> an ARP packet is broadcasted to all stations asking for the MAC address
>>>>>>> corresponding with the IPv4. The desired station responds with an ARP
>>>>>>> reply, and the ARP reply causes the switch to learn which port the
>>>>>>> station is connected to.
>>>>>>>
>>>>>>> With IPv6 networking, the situation is rather different. Rather than
>>>>>>> broadcasting ARP packets, a "neighbour solicitation" is multicasted
>>>>>>> rather than broadcasted. This multicast needs to reach the intended
>>>>>>> station in order for the neighbour to be discovered.
>>>>>>>
>>>>>>> Once a neighbour has been discovered, and entered into the sending
>>>>>>> stations neighbour cache, communication can restart at a point later
>>>>>>> without sending a new neighbour solicitation, even if the entry in
>>>>>>> the neighbour cache is marked as stale. This can be after the MAC
>>>>>>> address has expired from the forwarding cache of the DSA switch -
>>>>>>> when that occurs, there is a long pause in communication.
>>>>>>>
>>>>>>> Our DSA implementation for mv88e6xxx switches has defaulted to having
>>>>>>> multicast and unicast flooding disabled. As per the above description,
>>>>>>> this is fine for IPv4 networking, since the broadcasted ARP queries
>>>>>>> will be sent to and received by all stations on the same network.
>>>>>>> However, this breaks IPv6 very badly - blocking neighbour solicitations
>>>>>>> and later causing connections to stall.
>>>>>>>
>>>>>>> The defaults that the Linux bridge code expect from bridges are that
>>>>>>> unknown unicast frames and unknown multicast frames are flooded to
>>>>>>> all stations, which is at odds to the defaults adopted by our DSA
>>>>>>> implementation for mv88e6xxx switches.
>>>>>>>
>>>>>>> This commit enables by default flooding of both unknown unicast and
>>>>>>> unknown multicast frames. This means that mv88e6xxx DSA switches now
>>>>>>> behave as per the bridge(8) man page, and IPv6 works flawlessly through
>>>>>>> such a switch.
>>>>>>
>>>>>> Note that there is the open question whether this affects the case where
>>>>>> each port is used as a separate network interface: that case has not yet
>>>>>> been tested.
>>>>>
>>>>> I've checked with a mv88e6131 on the clearfog gt8k board. lan1
>>>>> connected to my lan with plenty of traffic on, and configured as
>>>>> part of a bridge. lan2 connected to the zii board, but not part
>>>>> of the bridge. Monitoring lan2 from the zii board shows no traffic
>>>>> that was received from lan1.
>>>>>
>>>>> So it looks fine.
>>>>
>>>> With the current state whereby we do not have the necessary hooks to
>>>> perform filtering on non-bridged/standalone ports, this is entirely fine
>>>> indeed.
>>>>
>>>> In the future this is part of something I want to address because it is
>>>> IMHO highly undesirable to have non-bridged ports be flooded with
>>>> unknown multicast or unknown unicast for that matter because that makes
>>>> them deviate from a standard NIC interface. Unknown unicast is not
>>>> necessarily a low hanging fruit, but still, if we have switches capable
>>>> of filtering, we might as well make use of that. Of course, one
>>>> difficulty is that we must not break running tcpdump on those DSA slave
>>>> network interfaces.
>>>
>>> Sorry, I think you have the wrong end of the stick.
>>>
>>> For a non-bridged port, I am seeing _no_ traffic apart from that
>>> explicitly sent out through that port. In other words, there are
>>> _no_ flooded frames coming out of the non-bridged port.
>>>
>>> This patch appears to have no material effect on non-bridged ports.
>>
>> Presumably because that non-bridged port and the CPU port are part of
>> the same domain with only those 2 ports and that is what we want.
>>
>> Now what happens if say you have a station that sends multicast traffic
>> through that port to e.g.: 226.94.1.1, I bet that port happily sends
>> that multicast traffic to the CPU port with no filtering what so ever
>> and this ends-up being dropped in the network stack because there is a
>> socket look up failure there. IMHO unless you have a receiver/server on
>> that network interface on the DSA network interface and a matching
>> socket you should not be receiving that multicast traffic and the switch
>> should be filtering it. Since the network stack will call into
>> ndo_set_rx_mode() for those cases, we really just need to make that
>> multicast traffic known, instead of unknown to the switch.
>
> If the port is not bridged, then it's operating as network interface,
> and traffic to/from that port needs to be routed to the CPU port so
> that it appears as it would do from a real network interface.
> Doing anything else makes breaks the idea that you can use a set
> of DSA ports as individual interfaces and run anything but IPv4
> non-multicast over them.
I am not proposing changing any of that just making use of the switch's
ability to snoop management traffic (ARP, DHCP, MLD, etc.) and use of
the network stack's hints in order to avoid flooding unknown multicast.
This is starting to diverge a little bit from the original issue though.
--
Florian
^ permalink raw reply
* Re: [BUG] [FIX] net: dsa: oops in br_vlan_enabled
From: Florian Fainelli @ 2019-02-17 22:38 UTC (permalink / raw)
To: Frank Wunderlich, netdev, andrew, Vivien Didelot
In-Reply-To: <trinity-1108d2b6-42f0-4770-b561-6a66804f210a-1550430319767@3c-app-gmx-bs61>
On 2/17/2019 11:05 AM, Frank Wunderlich wrote:
> Hi Florian
>
> a user from Bananapi-forum has reported the oops and i had reproduced this and patched out this oops.
> That means not that vlan in bridge works, only no crash.
>
> my kernel only conatins the second gmac dsa-patches for mt7530/bpi-r2 i've posted at the end of last year, but the oops also occur without them.
>
> have not changed bridge vlan-configuration by
>
> echo 1 > /sys/class/net/bridge_name/bridge/vlan_filtering
>
> maybe that vlan-objects/configuration is not passed through, but there should happen no oops.
>
> kernel-source is here: https://github.com/frank-w/BPI-R2-4.14/tree/4.19-main
>
> ps: please take me in CC so i can answer directly
You were in the To: of my previous reply, let's move it back there since
it has all context.
--
Florian
^ permalink raw reply
* [PATCH] lib/test_rhashtable: fix spelling mistake "existant" -> "existent"
From: Colin King @ 2019-02-17 22:52 UTC (permalink / raw)
To: Thomas Graf, Herbert Xu, netdev; +Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
There are spelling mistakes in warning macro messages. Fix them.
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
lib/test_rhashtable.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/test_rhashtable.c b/lib/test_rhashtable.c
index e52f8cafe227..2c0c53a99734 100644
--- a/lib/test_rhashtable.c
+++ b/lib/test_rhashtable.c
@@ -395,7 +395,7 @@ static int __init test_rhltable(unsigned int entries)
if (WARN(err, "cannot remove element at slot %d", i))
continue;
} else {
- if (WARN(err != -ENOENT, "removed non-existant element %d, error %d not %d",
+ if (WARN(err != -ENOENT, "removed non-existent element %d, error %d not %d",
i, err, -ENOENT))
continue;
}
@@ -440,7 +440,7 @@ static int __init test_rhltable(unsigned int entries)
if (WARN(err, "cannot remove element at slot %d", i))
continue;
} else {
- if (WARN(err != -ENOENT, "removed non-existant element, error %d not %d",
+ if (WARN(err != -ENOENT, "removed non-existent element, error %d not %d",
err, -ENOENT))
continue;
}
--
2.20.1
^ permalink raw reply related
* [PATCH] net/mlx4_en: fix spelling mistake: "quiting" -> "quitting"
From: Colin King @ 2019-02-17 23:03 UTC (permalink / raw)
To: Tariq Toukan, David S . Miller, netdev, linux-rdma
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
There is a spelling mistake in a en_err error message. Fix it.
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index 6b88881b8e35..c1438ae52a11 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -3360,7 +3360,7 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port,
dev->addr_len = ETH_ALEN;
mlx4_en_u64_to_mac(dev->dev_addr, mdev->dev->caps.def_mac[priv->port]);
if (!is_valid_ether_addr(dev->dev_addr)) {
- en_err(priv, "Port: %d, invalid mac burned: %pM, quiting\n",
+ en_err(priv, "Port: %d, invalid mac burned: %pM, quitting\n",
priv->port, dev->dev_addr);
err = -EINVAL;
goto out;
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v3 perf,bpf 05/11] perf, bpf: save bpf_prog_info in a rbtree in perf_env
From: Jiri Olsa @ 2019-02-17 23:05 UTC (permalink / raw)
To: Song Liu
Cc: netdev, linux-kernel, ast, daniel, kernel-team, peterz, acme,
jolsa, namhyung
In-Reply-To: <20190215215354.3114006-6-songliubraving@fb.com>
On Fri, Feb 15, 2019 at 01:53:48PM -0800, Song Liu wrote:
SNIP
> info_linear = bpf_program__get_prog_info_linear(fd, arrays);
> if (IS_ERR_OR_NULL(info_linear)) {
> @@ -151,8 +165,8 @@ static int perf_event__synthesize_one_bpf_prog(struct perf_tool *tool,
> machine, process);
> }
>
> - /* Synthesize PERF_RECORD_BPF_EVENT */
> if (opts->bpf_event) {
> + /* Synthesize PERF_RECORD_BPF_EVENT */
> *bpf_event = (struct bpf_event){
> .header = {
> .type = PERF_RECORD_BPF_EVENT,
> @@ -165,6 +179,19 @@ static int perf_event__synthesize_one_bpf_prog(struct perf_tool *tool,
> memcpy(bpf_event->tag, info->tag, BPF_TAG_SIZE);
> memset((void *)event + event->header.size, 0, machine->id_hdr_size);
> event->header.size += machine->id_hdr_size;
> +
> + /* save bpf_prog_info to env */
> + info_node = malloc(sizeof(struct bpf_prog_info_node));
> + if (info_node) {
> + info_node->info_linear = info_linear;
> + perf_env__insert_bpf_prog_info(env, info_node);
> + info_linear = NULL;
> + }
what if the allocation fails? we don't care?
jirka
^ permalink raw reply
* Re: [PATCH v3 perf,bpf 10/11] perf, bpf: enable annotation of bpf program
From: Jiri Olsa @ 2019-02-17 23:05 UTC (permalink / raw)
To: Song Liu
Cc: netdev, linux-kernel, ast, daniel, kernel-team, peterz, acme,
jolsa, namhyung
In-Reply-To: <20190215215354.3114006-11-songliubraving@fb.com>
On Fri, Feb 15, 2019 at 01:53:53PM -0800, Song Liu wrote:
SNIP
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index 70de8f6b3aee..078017d31ca9 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -22,6 +22,7 @@
> #include "annotate.h"
> #include "evsel.h"
> #include "evlist.h"
> +#include "bpf-event.h"
> #include "block-range.h"
> #include "string2.h"
> #include "arch/common.h"
> @@ -29,6 +30,9 @@
> #include <pthread.h>
> #include <linux/bitops.h>
> #include <linux/kernel.h>
> +#include <bfd.h>
> +#include <dis-asm.h>
> +#include <bpf/libbpf.h>
>
> /* FIXME: For the HE_COLORSET */
> #include "ui/browser.h"
> @@ -1672,6 +1676,147 @@ static int dso__disassemble_filename(struct dso *dso, char *filename, size_t fil
> return 0;
> }
>
> +static void get_exec_path(char *tpath, size_t size)
> +{
> + const char *path = "/proc/self/exe";
> + ssize_t len;
> +
> + len = readlink(path, tpath, size - 1);
> + assert(len > 0);
> + tpath[len] = 0;
> +}
this is also used in write_cmdline, could you please
move it under util.c and use it in write_cmdline as well?
thanks,
jirka
> +
> +static int symbol__disassemble_bpf(struct symbol *sym,
> + struct annotate_args *args)
> +{
> + struct annotation *notes = symbol__annotation(sym);
> + struct annotation_options *opts = args->options;
> + struct bpf_prog_info_linear *info_linear;
> + struct bpf_prog_linfo *prog_linfo = NULL;
> + struct bpf_prog_info_node *info_node;
> + int len = sym->end - sym->start;
> + disassembler_ftype disassemble;
> + struct map *map = args->ms.map;
> + struct disassemble_info info;
> + struct dso *dso = map->dso;
> + int pc = 0, count, sub_id;
> + struct btf *btf = NULL;
> + char tpath[PATH_MAX];
> + size_t buf_size;
> + int nr_skip = 0;
> + __u64 arrays;
> + char *buf;
> + bfd *bfdf;
> + FILE *s;
nice triangle ;-)
SNIP
^ permalink raw reply
* Re: [PATCH v3 perf,bpf 06/11] perf, bpf: save bpf_prog_info information as headers to perf.data
From: Jiri Olsa @ 2019-02-17 23:05 UTC (permalink / raw)
To: Song Liu
Cc: netdev, linux-kernel, ast, daniel, kernel-team, peterz, acme,
jolsa, namhyung
In-Reply-To: <20190215215354.3114006-7-songliubraving@fb.com>
On Fri, Feb 15, 2019 at 01:53:49PM -0800, Song Liu wrote:
SNIP
> +static int process_bpf_prog_info(struct feat_fd *ff,
> + void *data __maybe_unused)
> +{
> + struct bpf_prog_info_linear *info_linear;
> + struct bpf_prog_info_node *info_node;
> + struct perf_env *env = &ff->ph->env;
> + u32 count, i;
> + int err = -1;
> +
> + if (do_read_u32(ff, &count))
> + return -1;
> +
> + down_write(&env->bpf_progs.bpf_info_lock);
> +
> + for (i = 0; i < count; ++i) {
> + u32 info_len, data_len;
> +
> + info_linear = NULL;
> + info_node = NULL;
> + if (do_read_u32(ff, &info_len))
> + goto out;
> + if (do_read_u32(ff, &data_len))
> + goto out;
> +
> + if (info_len > sizeof(struct bpf_prog_info)) {
> + pr_warning("detected invalid bpf_prog_info\n");
> + goto out;
> + }
> +
> + info_linear = malloc(sizeof(struct bpf_prog_info_linear) +
> + data_len);
> + if (!info_linear)
> + goto out;
> + info_linear->info_len = sizeof(struct bpf_prog_info);
> + info_linear->data_len = data_len;
> + if (do_read_u64(ff, (u64 *)(&info_linear->arrays)))
> + goto out;
> + if (__do_read(ff, &info_linear->info, info_len))
> + goto out;
hum, hows the endianity swap handled for struct bpf_prog_info?
> + if (info_len < sizeof(struct bpf_prog_info))
> + memset(((void *)(&info_linear->info)) + info_len, 0,
> + sizeof(struct bpf_prog_info) - info_len);
> +
> + if (__do_read(ff, info_linear->data, data_len))
> + goto out;
and the data?
thanks,
jirka
^ permalink raw reply
* Re: [PATCH v3 perf,bpf 08/11] perf, bpf: save btf information as headers to perf.data
From: Jiri Olsa @ 2019-02-17 23:05 UTC (permalink / raw)
To: Song Liu
Cc: netdev, linux-kernel, ast, daniel, kernel-team, peterz, acme,
jolsa, namhyung
In-Reply-To: <20190215215354.3114006-9-songliubraving@fb.com>
On Fri, Feb 15, 2019 at 01:53:51PM -0800, Song Liu wrote:
SNIP
> struct header_print_data {
> diff --git a/tools/perf/util/header.h b/tools/perf/util/header.h
> index 0785c91b4c3a..ba51d8e43c53 100644
> --- a/tools/perf/util/header.h
> +++ b/tools/perf/util/header.h
> @@ -40,6 +40,7 @@ enum {
> HEADER_MEM_TOPOLOGY,
> HEADER_CLOCKID,
> HEADER_BPF_PROG_INFO,
> + HEADER_BTF,
hu, could we maybe prefix all the bpf related features with BPF_ ?
jirka
> HEADER_LAST_FEATURE,
> HEADER_FEAT_BITS = 256,
> };
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH v3 perf,bpf 05/11] perf, bpf: save bpf_prog_info in a rbtree in perf_env
From: Jiri Olsa @ 2019-02-17 23:05 UTC (permalink / raw)
To: Song Liu
Cc: netdev, linux-kernel, ast, daniel, kernel-team, peterz, acme,
jolsa, namhyung
In-Reply-To: <20190215215354.3114006-6-songliubraving@fb.com>
On Fri, Feb 15, 2019 at 01:53:48PM -0800, Song Liu wrote:
SNIP
> diff --git a/tools/perf/util/env.h b/tools/perf/util/env.h
> index d01b8355f4ca..d0c53fe6d431 100644
> --- a/tools/perf/util/env.h
> +++ b/tools/perf/util/env.h
> @@ -3,7 +3,9 @@
> #define __PERF_ENV_H
>
> #include <linux/types.h>
> +#include <linux/rbtree.h>
> #include "cpumap.h"
> +#include "rwsem.h"
>
> struct cpu_topology_map {
> int socket_id;
> @@ -64,8 +66,19 @@ struct perf_env {
> struct memory_node *memory_nodes;
> unsigned long long memory_bsize;
> u64 clockid_res_ns;
> +
> + /*
> + * bpf_info_lock protects bpf rbtrees. This is needed because the
> + * trees are accessed by different threads in perf-top
> + */
> + struct {
> + struct rw_semaphore bpf_info_lock;
> + struct rb_root bpf_prog_infos;
there's already struct name 'bpf_progs', no need for
those bpf_ prefixes
jirka
> + } bpf_progs;
SNIP
^ permalink raw reply
* Re: [PATCH v3 perf,bpf 11/11] perf, bpf: save information about short living bpf programs
From: Jiri Olsa @ 2019-02-17 23:05 UTC (permalink / raw)
To: Song Liu
Cc: netdev, linux-kernel, ast, daniel, kernel-team, peterz, acme,
jolsa, namhyung
In-Reply-To: <20190215215354.3114006-12-songliubraving@fb.com>
On Fri, Feb 15, 2019 at 01:53:54PM -0800, Song Liu wrote:
> To annotate bpf programs in perf, it is necessary to save information in
> bpf_prog_info and btf. For short living bpf program, it is necessary to
> save these information before it is unloaded.
>
> This patch saves these information in a separate thread. This thread
> creates its own evlist, that only tracks bpf events. This evlists uses
> ring buffer with very low watermark for lower latency. When bpf load
> events are received, this thread tries to gather information via sys_bpf
> and save it in perf_env.
could we make this a generic? like havving support
to create a thread that would process sideband evets?
itcould be used for the that buildid stuff and for
things like getting full command lines from comm
events and such
jirka
^ permalink raw reply
* Re: [PATCH v3 perf,bpf 10/11] perf, bpf: enable annotation of bpf program
From: Jiri Olsa @ 2019-02-17 23:06 UTC (permalink / raw)
To: Song Liu
Cc: netdev, linux-kernel, ast, daniel, kernel-team, peterz, acme,
jolsa, namhyung
In-Reply-To: <20190215215354.3114006-11-songliubraving@fb.com>
On Fri, Feb 15, 2019 at 01:53:53PM -0800, Song Liu wrote:
> This patch enables the annotation of bpf program.
>
> A new dso type DSO_BINARY_TYPE__BPF_PROG_INFO is introduced to for BPF
> programs. In symbol__disassemble(), DSO_BINARY_TYPE__BPF_PROG_INFO dso
> calls into a new function symbol__disassemble_bpf(), where annotation
> line information is filled based bpf_prog_info and btf saved in given
> perf_env.
>
> symbol__disassemble_bpf() uses libbfd to disassemble bpf programs.
>
> Signed-off-by: Song Liu <songliubraving@fb.com>
> ---
> tools/perf/Makefile.config | 6 +-
> tools/perf/util/annotate.c | 149 +++++++++++++++++++++++++++++++++++-
> tools/perf/util/bpf-event.c | 48 ++++++++++++
> tools/perf/util/bpf-event.h | 4 +
> tools/perf/util/dso.c | 1 +
> tools/perf/util/dso.h | 33 +++++---
> tools/perf/util/symbol.c | 1 +
> 7 files changed, 229 insertions(+), 13 deletions(-)
>
> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> index b441c88cafa1..ab223239f1fb 100644
> --- a/tools/perf/Makefile.config
> +++ b/tools/perf/Makefile.config
> @@ -701,7 +701,7 @@ else
> endif
>
> ifeq ($(feature-libbfd), 1)
> - EXTLIBS += -lbfd
> + EXTLIBS += -lbfd -lopcodes
hum, what's this -lopcodes lib? please add it in a separate
patch and explain why it's added and how it affects others
it looks like it could break some distros without that lib
jirka
^ permalink raw reply
* Re: [PATCH v3 perf,bpf 10/11] perf, bpf: enable annotation of bpf program
From: Jiri Olsa @ 2019-02-17 23:06 UTC (permalink / raw)
To: Song Liu
Cc: netdev, linux-kernel, ast, daniel, kernel-team, peterz, acme,
jolsa, namhyung
In-Reply-To: <20190215215354.3114006-11-songliubraving@fb.com>
On Fri, Feb 15, 2019 at 01:53:53PM -0800, Song Liu wrote:
SNIP
> +static int symbol__disassemble_bpf(struct symbol *sym,
> + struct annotate_args *args)
> +{
> + struct annotation *notes = symbol__annotation(sym);
> + struct annotation_options *opts = args->options;
> + struct bpf_prog_info_linear *info_linear;
> + struct bpf_prog_linfo *prog_linfo = NULL;
> + struct bpf_prog_info_node *info_node;
> + int len = sym->end - sym->start;
> + disassembler_ftype disassemble;
> + struct map *map = args->ms.map;
> + struct disassemble_info info;
> + struct dso *dso = map->dso;
> + int pc = 0, count, sub_id;
> + struct btf *btf = NULL;
> + char tpath[PATH_MAX];
> + size_t buf_size;
> + int nr_skip = 0;
> + __u64 arrays;
> + char *buf;
> + bfd *bfdf;
> + FILE *s;
> +
> + if (dso->binary_type != DSO_BINARY_TYPE__BPF_PROG_INFO)
> + return -1;
> +
> + pr_debug("%s: handling sym %s addr %lx len %lx\n", __func__,
> + sym->name, sym->start, sym->end - sym->start);
> +
> + memset(tpath, 0, sizeof(tpath));
> + get_exec_path(tpath, sizeof(tpath));
> +
> + bfdf = bfd_openr(tpath, NULL);
> + assert(bfdf);
> + assert(bfd_check_format(bfdf, bfd_object));
> +
> + s = open_memstream(&buf, &buf_size);
what if open_memstream fails?
> + init_disassemble_info(&info, s,
> + (fprintf_ftype) fprintf);
> +
> + info.arch = bfd_get_arch(bfdf);
> + info.mach = bfd_get_mach(bfdf);
> +
> + arrays = 1UL << BPF_PROG_INFO_JITED_INSNS;
> + arrays |= 1UL << BPF_PROG_INFO_JITED_KSYMS;
> + arrays |= 1UL << BPF_PROG_INFO_LINE_INFO;
> + arrays |= 1UL << BPF_PROG_INFO_FUNC_INFO;
what's the arrays for?
> +
> + info_node = perf_env__find_bpf_prog_info(dso->bpf_prog.env,
> + dso->bpf_prog.id);
> + if (!info_node)
> + return -1;
> + info_linear = info_node->info_linear;
> + sub_id = dso->bpf_prog.sub_id;
> +
> + info.buffer = (void *)(info_linear->info.jited_prog_insns);
> + info.buffer_length = info_linear->info.jited_prog_len;
> +
> + if (info_linear->info.nr_line_info)
> + prog_linfo = bpf_prog_linfo__new(&info_linear->info);
> + prog_linfo = prog_linfo;
> +
> + if (info_linear->info.btf_id) {
> + struct btf_node *node;
> +
> + node = perf_env__find_btf(dso->bpf_prog.env,
> + info_linear->info.btf_id);
> + if (node)
> + btf = btf__new((__u8 *)(node->data),
> + node->data_size);
> + }
what if btf__new fails? the btf__name_by_offset
does not check btf != NULL
> +
> + disassemble_init_for_target(&info);
> +
> +#ifdef DISASM_FOUR_ARGS_SIGNATURE
> + disassemble = disassembler(info.arch,
> + bfd_big_endian(bfdf),
> + info.mach,
> + bfdf);
> +#else
> + disassemble = disassembler(bfdf);
> +#endif
> + assert(disassemble);
> +
> + fflush(s);
any chance this function could be split into some logical
pieces/fucntions?
thanks,
jirka
> + do {
> + const struct bpf_line_info *linfo = NULL;
> + struct disasm_line *dl;
> + size_t prev_buf_size;
> + const char *srcline;
> + u64 addr;
> +
> + addr = pc + ((u64 *)(info_linear->info.jited_ksyms))[sub_id];
> + count = disassemble(pc, &info);
> +
> + linfo = bpf_prog_linfo__lfind_addr_func(prog_linfo, addr, sub_id,
> + nr_skip);
> +
> + if (linfo) {
> + srcline = btf__name_by_offset(btf, linfo->line_off);
> + nr_skip++;
> + } else
> + srcline = NULL;
> +
> + fprintf(s, "\n");
> + prev_buf_size = buf_size;
> + fflush(s);
> +
> + if (!opts->hide_src_code && srcline) {
> + args->offset = -1;
> + args->line = strdup(srcline);
> + args->line_nr = 0;
> + args->ms.sym = sym;
> + dl = disasm_line__new(args);
> + annotation_line__add(&dl->al, ¬es->src->source);
> + }
> +
> + args->offset = pc;
> + args->line = buf + prev_buf_size;
> + args->line_nr = 0;
> + args->ms.sym = sym;
> + dl = disasm_line__new(args);
> + annotation_line__add(&dl->al, ¬es->src->source);
> +
> + pc += count;
> + } while (count > 0 && pc < len);
> +
> + bfd_close(bfdf);
> + return 0;
> +}
SNIP
^ 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