* Re: [PATCH bpf-next v3 00/13] tools: bpf: extend bpftool prog load
From: Daniel Borkmann @ 2018-07-11 20:18 UTC (permalink / raw)
To: Jakub Kicinski, alexei.starovoitov, Andrey Ignatov; +Cc: oss-drivers, netdev
In-Reply-To: <20180710214307.4834-1-jakub.kicinski@netronome.com>
On 07/10/2018 11:42 PM, Jakub Kicinski wrote:
> Hi!
>
> This series starts with two minor clean ups to test_offload.py
> selftest script.
>
> The next 11 patches extend the abilities of bpftool prog load
> beyond the simple cgroup use cases. Three new parameters are
> added:
>
> - type - allows specifying program type, independent of how
> code sections are named;
> - map - allows reusing existing maps, instead of creating a new
> map on every program load;
> - dev - offload/binding to a device.
>
> A number of changes to libbpf is required to accomplish the task.
> The section - program type logic mapping is exposed. We should
> probably aim to use the libbpf program section naming everywhere.
> For reuse of maps we need to allow users to set FD for bpf map
> object in libbpf.
>
> Examples
>
> Load program my_xdp.o and pin it as /sys/fs/bpf/my_xdp, for xdp
> program type:
>
> $ bpftool prog load my_xdp.o /sys/fs/bpf/my_xdp \
> type xdp
>
> As above but for offload:
>
> $ bpftool prog load my_xdp.o /sys/fs/bpf/my_xdp \
> type xdp \
> dev netdevsim0
>
> Load program my_maps.o, but for the first map reuse map id 17,
> and for the map called "other_map" reuse pinned map /sys/fs/bpf/map0:
>
> $ bpftool prog load my_maps.o /sys/fs/bpf/prog \
> map idx 0 id 17 \
> map name other_map pinned /sys/fs/bpf/map0
>
> ---
> v3:
> - fix return codes in patch 5;
> - rename libbpf_prog_type_by_string() -> libbpf_prog_type_by_name();
> - fold file path into xattr in patch 8;
> - add patch 10;
> - use dup3() in patch 12;
> - depend on fd value in patch 12;
> - close old fd in patch 12.
> v2:
> - add compat for reallocarray().
Applied to bpf-next, thanks Jakub!
^ permalink raw reply
* you need photos
From: Julie Ryan @ 2018-07-11 13:49 UTC (permalink / raw)
To: netdev
We would like to introduce our image editing for you.
. Cut out, masking, clipping path, deep etching, transparent background
Colour correction,
black and white, light and shadows etc.
. Dust cleaning, spot cleaning
. Beauty retouching, skin retouching, face retouching, body retouching
. Product image Retouching
. Wedding & Event Album Design.
. Portrait image Retouching
We give you editing test on your photos.
Thanks,
Julie Ryan
^ permalink raw reply
* [PATCH net-next 0/2] net: phy: add functionality to speed down PHY when waiting for WoL packet
From: Heiner Kallweit @ 2018-07-11 20:29 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev@vger.kernel.org
Some network drivers include functionality to speed down the PHY when
suspending and just waiting for a WoL packet because this saves energy.
This patch is based on our recent discussion about factoring out this
functionality to phylib. First user will be the r8169 driver.
Heiner Kallweit (2):
net: phy: add helper phy_config_aneg
net: phy: add phy_speed_down and phy_speed_up
drivers/net/phy/phy.c | 91 +++++++++++++++++++++++++++++++++++++++++--
include/linux/phy.h | 2 +
2 files changed, 89 insertions(+), 4 deletions(-)
--
2.18.0
^ permalink raw reply
* [PATCH net-next 1/2] net: phy: add helper phy_config_aneg
From: Heiner Kallweit @ 2018-07-11 20:30 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <0d031081-4a7f-ddde-87c0-2c1c6be543c3@gmail.com>
This functionality will also be needed in subsequent patches of this
series, therefore factor it out to a helper.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/phy/phy.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 537297d2..c4aa360d 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -467,6 +467,14 @@ int phy_mii_ioctl(struct phy_device *phydev, struct ifreq *ifr, int cmd)
}
EXPORT_SYMBOL(phy_mii_ioctl);
+static int phy_config_aneg(struct phy_device *phydev)
+{
+ if (phydev->drv->config_aneg)
+ return phydev->drv->config_aneg(phydev);
+ else
+ return genphy_config_aneg(phydev);
+}
+
/**
* phy_start_aneg_priv - start auto-negotiation for this PHY device
* @phydev: the phy_device struct
@@ -493,10 +501,7 @@ static int phy_start_aneg_priv(struct phy_device *phydev, bool sync)
/* Invalidate LP advertising flags */
phydev->lp_advertising = 0;
- if (phydev->drv->config_aneg)
- err = phydev->drv->config_aneg(phydev);
- else
- err = genphy_config_aneg(phydev);
+ err = phy_config_aneg(phydev);
if (err < 0)
goto out_unlock;
--
2.18.0
^ permalink raw reply related
* [PATCH net-next 2/2] net: phy: add phy_speed_down and phy_speed_up
From: Heiner Kallweit @ 2018-07-11 20:31 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <0d031081-4a7f-ddde-87c0-2c1c6be543c3@gmail.com>
Some network drivers include functionality to speed down the PHY when
suspending and just waiting for a WoL packet because this saves energy.
This functionality is quite generic, therefore let's factor it out to
phylib.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/phy/phy.c | 78 +++++++++++++++++++++++++++++++++++++++++++
include/linux/phy.h | 2 ++
2 files changed, 80 insertions(+)
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index c4aa360d..0547c603 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -551,6 +551,84 @@ int phy_start_aneg(struct phy_device *phydev)
}
EXPORT_SYMBOL(phy_start_aneg);
+static int phy_poll_aneg_done(struct phy_device *phydev)
+{
+ unsigned int retries = 100;
+ int ret;
+
+ do {
+ msleep(100);
+ ret = phy_aneg_done(phydev);
+ } while (!ret && --retries);
+
+ if (!ret)
+ return -ETIMEDOUT;
+
+ return ret < 0 ? ret : 0;
+}
+
+/**
+ * phy_speed_down - set speed to lowest speed supported by both link partners
+ * @phydev: the phy_device struct
+ * @sync: perform action synchronously
+ *
+ * Description: Typically used to save energy when waiting for a WoL packet
+ */
+int phy_speed_down(struct phy_device *phydev, bool sync)
+{
+ u32 adv = phydev->lp_advertising & phydev->supported;
+ u32 adv_old = phydev->advertising;
+ int ret;
+
+ if (phydev->autoneg != AUTONEG_ENABLE)
+ return 0;
+
+ if (adv & PHY_10BT_FEATURES)
+ phydev->advertising &= ~(PHY_100BT_FEATURES |
+ PHY_1000BT_FEATURES);
+ else if (adv & PHY_100BT_FEATURES)
+ phydev->advertising &= ~PHY_1000BT_FEATURES;
+
+ if (phydev->advertising == adv_old)
+ return 0;
+
+ ret = phy_config_aneg(phydev);
+ if (ret)
+ return ret;
+
+ return sync ? phy_poll_aneg_done(phydev) : 0;
+}
+EXPORT_SYMBOL_GPL(phy_speed_down);
+
+/**
+ * phy_speed_up - (re)set advertised speeds to all supported speeds
+ * @phydev: the phy_device struct
+ * @sync: perform action synchronously
+ *
+ * Description: Used to revert the effect of phy_speed_down
+ */
+int phy_speed_up(struct phy_device *phydev, bool sync)
+{
+ u32 mask = PHY_10BT_FEATURES | PHY_100BT_FEATURES | PHY_1000BT_FEATURES;
+ u32 adv_old = phydev->advertising;
+ int ret;
+
+ if (phydev->autoneg != AUTONEG_ENABLE)
+ return 0;
+
+ phydev->advertising = (adv_old & ~mask) | (phydev->supported & mask);
+
+ if (phydev->advertising == adv_old)
+ return 0;
+
+ ret = phy_config_aneg(phydev);
+ if (ret)
+ return ret;
+
+ return sync ? phy_poll_aneg_done(phydev) : 0;
+}
+EXPORT_SYMBOL_GPL(phy_speed_up);
+
/**
* phy_start_machine - start PHY state machine tracking
* @phydev: the phy_device struct
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 6cd09098..275f528e 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -942,6 +942,8 @@ void phy_start(struct phy_device *phydev);
void phy_stop(struct phy_device *phydev);
int phy_start_aneg(struct phy_device *phydev);
int phy_aneg_done(struct phy_device *phydev);
+int phy_speed_down(struct phy_device *phydev, bool sync);
+int phy_speed_up(struct phy_device *phydev, bool sync);
int phy_stop_interrupts(struct phy_device *phydev);
int phy_restart_aneg(struct phy_device *phydev);
--
2.18.0
^ permalink raw reply related
* Re: [PATCH iproute2-next] ipaddress: fix label matching
From: Vincent Bernat @ 2018-07-11 20:26 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, serhe.popovych
In-Reply-To: <20180711130328.1d5bc82f@xeon-e3>
❦ 11 juillet 2018 13:03 -0700, Stephen Hemminger <stephen@networkplumber.org> :
>> Since 9516823051ce, "ip addr show label lo:1" doesn't work
>> anymore (doesn't show any address, despite a matching label).
>> Reverting to return 0 instead of -1 fix the issue.
>>
>> However, the condition says: "if we filter by label [...] and the
>> label does NOT match the interface name". This makes little sense to
>> compare the label with the interface name. There is also a logic
>> around filter family being provided or not. The match against the
>> label is done by ifa_label_match_rta() in print_addrinfo() and
>> ipaddr_filter().
>>
>> Just removing the condition makes "ip addr show" works as expected
>> with or without specifying a label, both when the label is matching
>> and not matching. It also works if we specify a label and the label is
>> the interface name. The flush operation also works as expected.
>>
>> Fixes: 9516823051ce ("ipaddress: Improve print_linkinfo()")
>> Signed-off-by: Vincent Bernat <vincent@bernat.im>
>> ---
>> ip/ipaddress.c | 5 -----
>> 1 file changed, 5 deletions(-)
>>
>> diff --git a/ip/ipaddress.c b/ip/ipaddress.c
>> index 5009bfe6d2e3..20ef6724944e 100644
>> --- a/ip/ipaddress.c
>> +++ b/ip/ipaddress.c
>> @@ -837,11 +837,6 @@ int print_linkinfo(const struct sockaddr_nl *who,
>> if (!name)
>> return -1;
>>
>> - if (filter.label &&
>> - (!filter.family || filter.family == AF_PACKET) &&
>> - fnmatch(filter.label, name, 0))
>> - return -1;
>> -
>> if (tb[IFLA_GROUP]) {
>> int group = rta_getattr_u32(tb[IFLA_GROUP]);
>>
>
> If this is a regression, it should go to iproute2 not iproute2-next.
>
> Surprised by the solution since it is removing code that was there
> before the commit you referenced in Fixes.
Yes, but as I explain in the commit message, the condition does not make
sense for me: why would we match the label against the interface name?
This code exists since a long time.
--
The lunatic, the lover, and the poet,
Are of imagination all compact...
-- Wm. Shakespeare, "A Midsummer Night's Dream"
^ permalink raw reply
* Re: [PATCH bpf-next] bpf: better availability probing for seg6 helpers
From: Daniel Borkmann @ 2018-07-11 20:35 UTC (permalink / raw)
To: Mathieu Xhonneux, netdev; +Cc: alexei.starovoitov
In-Reply-To: <5da76612-54b4-d616-6def-a103cafee074@iogearbox.net>
On 07/10/2018 09:20 PM, Daniel Borkmann wrote:
> On 07/10/2018 06:54 PM, Mathieu Xhonneux wrote:
>> bpf_lwt_seg6_* helpers require CONFIG_IPV6_SEG6_BPF, and currently
>> return -EOPNOTSUPP to indicate unavailability. This patch forces the
>> BPF verifier to reject programs using these helpers when
>> !CONFIG_IPV6_SEG6_BPF, allowing users to more easily probe if they are
>> available or not.
>>
>> Signed-off-by: Mathieu Xhonneux <m.xhonneux@gmail.com>
>
> Note, just fyi, this would need to go to bpf tree (and not bpf-next) as
> otherwise there's a change in behavior.
Applied, thanks Mathieu!
^ permalink raw reply
* Re: [PATCH net-next] tc-testing: add geneve options in tunnel_key unit tests
From: Lucas Bates @ 2018-07-11 20:37 UTC (permalink / raw)
To: Jakub Kicinski
Cc: David Miller, Keara Leibovitz, Roman Mashak, oss-drivers,
Linux Kernel Network Developers, Pieter Jansen van Vuuren
In-Reply-To: <20180711012231.20538-1-jakub.kicinski@netronome.com>
On Tue, Jul 10, 2018 at 9:22 PM, Jakub Kicinski
<jakub.kicinski@netronome.com> wrote:
> From: Pieter Jansen van Vuuren <pieter.jansenvanvuuren@netronome.com>
>
> Extend tc tunnel_key action unit tests with geneve options. Tests
> include testing single and multiple geneve options, as well as
> testing geneve options that are expected to fail.
>
> Signed-off-by: Pieter Jansen van Vuuren <pieter.jansenvanvuuren@netronome.com>
Acked-by: Lucas Bates <lucasb@mojatatu.com>
^ permalink raw reply
* Re: [PATCH net-next 1/2] net: phy: add helper phy_config_aneg
From: Andrew Lunn @ 2018-07-11 20:38 UTC (permalink / raw)
To: Heiner Kallweit; +Cc: Florian Fainelli, David Miller, netdev@vger.kernel.org
In-Reply-To: <cff2a19d-09c1-43fe-ab9a-ce1631d5c18c@gmail.com>
On Wed, Jul 11, 2018 at 10:30:27PM +0200, Heiner Kallweit wrote:
> This functionality will also be needed in subsequent patches of this
> series, therefore factor it out to a helper.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH net-next v19 0/8] sched: Add Common Applications Kept Enhanced (cake) qdisc
From: Toke Høiland-Jørgensen @ 2018-07-11 20:40 UTC (permalink / raw)
To: David Miller
Cc: netdev, gamanakis, peteheist, ycheng, ncardwell, dave.taht,
netfilter-devel, cake
In-Reply-To: <20180710.225648.251006161729517659.davem@davemloft.net>
David Miller <davem@davemloft.net> writes:
> From: Toke Høiland-Jørgensen <toke@toke.dk>
> Date: Fri, 06 Jul 2018 17:37:19 +0200
>
>> This patch series adds the CAKE qdisc, and has been split up to ease
>> review.
>>
>> I have attempted to split out each configurable feature into its own patch.
>> The first commit adds the base shaper and packet scheduler, while
>> subsequent commits add the optional features. The full userspace API and
>> most data structures are included in this commit, but options not
>> understood in the base version will be ignored.
>>
>> The result of applying the entire series is identical to the out of tree
>> version that have seen extensive testing in previous deployments, most
>> notably as an out of tree patch to OpenWrt. However, note that I have only
>> compile tested the individual patches; so the whole series should be
>> considered as a unit.
>
> Ok, I decided to apply this even though there are still bits I'm not
> %100 happy with.
Yay, awesome, thanks! :)
> I don't like the netfilter dependency at all.
>
> You can get the NAT addresses in other ways as I've tried to suggest
> in the past. Your scheme absolutely does not work with act_nat in the
> packet scheduler, not any NAT done by XDP/eBPF programs.
Just to reiterate why we didn't go with your suggestion of recording the
pre-NAT IP in the flow dissector as the packet comes in:
- It only works on egress; on ingress (with an ifb), packets hit the
qdisc before NAT, so we need the stateful lookup in CAKE for this
case, which is a common deployment scenario.
- It's not needed for act_nat (for 1-to-1 NAT, hashing on the post-NAT
IP is fine), and it won't work for XDP (which would change the packets
before the flow dissector sees them). This means that custom NAT
solutions in TC BPF hooks are the only ones that would benefit; and
they can just set the classifier to achieve the same thing.
Now, I'm absolutely not opposed to having this as a fallback egress-only
mechanism. I might even be convinced to write it myself if someone
demonstrates that they really need it :)
-Toke
^ permalink raw reply
* Re: KASAN: slab-out-of-bounds Read in rds_cong_queue_updates (2)
From: Santosh Shilimkar @ 2018-07-11 21:00 UTC (permalink / raw)
To: syzbot, davem, linux-kernel, linux-rdma, netdev, rds-devel,
syzkaller-bugs
In-Reply-To: <0000000000005274c40570be9f48@google.com>
On 7/11/2018 12:55 PM, syzbot wrote:
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit: 0026129c8629 rhashtable: add restart routine in
> rhashtable..
> git tree: net
> console output: https://syzkaller.appspot.com/x/log.txt?x=10b7ced0400000
> kernel config: https://syzkaller.appspot.com/x/.config?x=b88de6eac8694da6
> dashboard link:
> https://syzkaller.appspot.com/bug?extid=0570fef57a5e020bdc87
> compiler: gcc (GCC) 8.0.1 20180413 (experimental)
>
> Unfortunately, I don't have any reproducer for this crash yet.
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+0570fef57a5e020bdc87@syzkaller.appspotmail.com
>
dup: syzbot+4c20b3866171ce8441d2@syzkaller.appspotmail.com
^ permalink raw reply
* Re: [PATCH net-next 2/2] net: phy: add phy_speed_down and phy_speed_up
From: Andrew Lunn @ 2018-07-11 20:55 UTC (permalink / raw)
To: Heiner Kallweit; +Cc: Florian Fainelli, David Miller, netdev@vger.kernel.org
In-Reply-To: <407ed2cd-db27-f179-8b98-0d1e61513e07@gmail.com>
> +/**
> + * phy_speed_down - set speed to lowest speed supported by both link partners
> + * @phydev: the phy_device struct
> + * @sync: perform action synchronously
> + *
> + * Description: Typically used to save energy when waiting for a WoL packet
> + */
> +int phy_speed_down(struct phy_device *phydev, bool sync)
This sync parameter needs some more thought. I'm not sure it is safe.
How does a PHY trigger a WoL wake up? I guess some use the interrupt
pin. How does a PHY indicate auto-neg has completed? It triggers an
interrupt. So it seems like there is a danger here we suspend, and
then wake up 2 seconds later when auto-neg has completed.
I'm not sure we can safely suspend until auto-neg has completed.
> +/**
> + * phy_speed_up - (re)set advertised speeds to all supported speeds
> + * @phydev: the phy_device struct
> + * @sync: perform action synchronously
> + *
> + * Description: Used to revert the effect of phy_speed_down
> + */
> +int phy_speed_up(struct phy_device *phydev, bool sync)
And here, i'm thinking the opposite. A MAC driver needs to be ready
for the PHY state to change at any time. So why do we need to wait?
Just let the normal mechanisms inform the MAC when the link is up.
Andrew
^ permalink raw reply
* Re: [PATCH net-next 1/2] net: phy: add helper phy_config_aneg
From: Florian Fainelli @ 2018-07-11 21:04 UTC (permalink / raw)
To: Heiner Kallweit, Andrew Lunn, David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <cff2a19d-09c1-43fe-ab9a-ce1631d5c18c@gmail.com>
On 07/11/2018 01:30 PM, Heiner Kallweit wrote:
> This functionality will also be needed in subsequent patches of this
> series, therefore factor it out to a helper.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* [PATCH] net: ethtool: fix spelling mistake: "tubale" -> "tunable"
From: Michael Heimpold @ 2018-07-11 21:10 UTC (permalink / raw)
To: netdev, davem; +Cc: linux-kernel, Michael Heimpold
Signed-off-by: Michael Heimpold <mhei@heimpold.de>
---
include/uapi/linux/ethtool.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index 4ca65b56084f..7363f18e65a5 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -226,7 +226,7 @@ enum tunable_id {
ETHTOOL_TX_COPYBREAK,
ETHTOOL_PFC_PREVENTION_TOUT, /* timeout in msecs */
/*
- * Add your fresh new tubale attribute above and remember to update
+ * Add your fresh new tunable attribute above and remember to update
* tunable_strings[] in net/core/ethtool.c
*/
__ETHTOOL_TUNABLE_COUNT,
--
2.17.1
^ permalink raw reply related
* Re: [PATCH net-next 2/2] net: phy: add phy_speed_down and phy_speed_up
From: Heiner Kallweit @ 2018-07-11 21:08 UTC (permalink / raw)
To: Andrew Lunn; +Cc: Florian Fainelli, David Miller, netdev@vger.kernel.org
In-Reply-To: <20180711205518.GJ21430@lunn.ch>
On 11.07.2018 22:55, Andrew Lunn wrote:
>> +/**
>> + * phy_speed_down - set speed to lowest speed supported by both link partners
>> + * @phydev: the phy_device struct
>> + * @sync: perform action synchronously
>> + *
>> + * Description: Typically used to save energy when waiting for a WoL packet
>> + */
>> +int phy_speed_down(struct phy_device *phydev, bool sync)
>
> This sync parameter needs some more thought. I'm not sure it is safe.
>
> How does a PHY trigger a WoL wake up? I guess some use the interrupt
> pin. How does a PHY indicate auto-neg has completed? It triggers an
> interrupt. So it seems like there is a danger here we suspend, and
> then wake up 2 seconds later when auto-neg has completed.
>
> I'm not sure we can safely suspend until auto-neg has completed.
>
>> +/**
>> + * phy_speed_up - (re)set advertised speeds to all supported speeds
>> + * @phydev: the phy_device struct
>> + * @sync: perform action synchronously
>> + *
>> + * Description: Used to revert the effect of phy_speed_down
>> + */
>> +int phy_speed_up(struct phy_device *phydev, bool sync)
>
> And here, i'm thinking the opposite. A MAC driver needs to be ready
> for the PHY state to change at any time. So why do we need to wait?
> Just let the normal mechanisms inform the MAC when the link is up.
>
I see your points, thanks for the feedback. In my case WoL triggers
a PCI PME and the code works as expected, but I agree this may be
different in other setups (external PHY).
The sync parameter was inspired by following comment from Florian:
"One thing that bothers me a bit is that this should ideally be
offered as both blocking and non-blocking options"
So let's see which comments he may have before preparing a v2.
> Andrew
>
Heiner
^ permalink raw reply
* Re: [PATCH bpf-next v4 2/3] bpf: btf: add btf print functionality
From: Okash Khawaja @ 2018-07-11 21:18 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Daniel Borkmann, Martin KaFai Lau, Alexei Starovoitov,
Yonghong Song, Quentin Monnet, David S. Miller, netdev,
kernel-team, linux-kernel
In-Reply-To: <20180711121015.42873aff@cakuba.lan>
On Wed, Jul 11, 2018 at 12:10:15PM -0700, Jakub Kicinski wrote:
> Thank you for all the changes made so far.
>
> On Tue, 10 Jul 2018 20:21:10 -0700, Okash Khawaja wrote:
> > --- /dev/null
> > +++ b/tools/bpf/bpftool/btf_dumper.c
> > @@ -0,0 +1,248 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Copyright (c) 2018 Facebook */
> > +
> > +#include <linux/btf.h>
> > +#include <linux/err.h>
> > +#include <stdio.h> /* for (FILE *) used by json_writer */
> > +#include <linux/bitops.h>
> > +#include <string.h>
> > +#include <ctype.h>
>
> Again, please sort the headers the way I suggested. Otherwise as the
> list of includes grows it's hard to know what's already there.
>
> > +#include "btf.h"
> > +#include "json_writer.h"
> > +#include "main.h"
> > +
> > +#define BITS_PER_BYTE_MASK (BITS_PER_BYTE - 1)
> > +#define BITS_PER_BYTE_MASKED(bits) ((bits) & BITS_PER_BYTE_MASK)
> > +#define BITS_ROUNDDOWN_BYTES(bits) ((bits) >> 3)
> > +#define BITS_ROUNDUP_BYTES(bits) \
> > + (BITS_ROUNDDOWN_BYTES(bits) + !!BITS_PER_BYTE_MASKED(bits))
> > +
> > +static int btf_dumper_do_type(const struct btf_dumper *d, __u32 type_id,
> > + __u8 bit_offset, const void *data);
> > +
> > +static void btf_dumper_ptr(const void *data, json_writer_t *jw,
> > + bool is_plain_text)
> > +{
> > + if (is_plain_text)
> > + jsonw_printf(jw, "%p", *((unsigned long *)data));
> > + else
> > + jsonw_printf(jw, "%u", *((unsigned long *)data));
>
> Again, please drop the extraneous parens.
>
> > +}
> > +
>
> > +static void btf_dumper_int_bits(__u32 int_type, __u8 bit_offset,
> > + const void *data, json_writer_t *jw,
> > + bool is_plain_text)
> > +{
> > + int left_shift_bits, right_shift_bits;
> > + int nr_bits = BTF_INT_BITS(int_type);
> > + int total_bits_offset;
> > + int bytes_to_copy;
> > + int bits_to_copy;
> > + __u64 print_num;
> > +
> > + total_bits_offset = bit_offset + BTF_INT_OFFSET(int_type);
> > + data += BITS_ROUNDDOWN_BYTES(total_bits_offset);
> > + bit_offset = BITS_PER_BYTE_MASKED(total_bits_offset);
> > + bits_to_copy = bit_offset + nr_bits;
> > + bytes_to_copy = BITS_ROUNDUP_BYTES(bits_to_copy);
> > +
> > + print_num = 0;
> > + memcpy(&print_num, data, bytes_to_copy);
> > +#ifdef __BIG_ENDIAN_BITFIELD
> > + left_shift_bits = bit_offset;
> > +#else
> > + left_shift_bits = 64 - bits_to_copy;
> > +#endif
> > + right_shift_bits = 64 - nr_bits;
>
> Please include <asm/byteorder.h> as I suggested to you previously.
> This is dead code right now, look:
Sorry, should have checked ifndef case. Will fix. Thanks.
>
> $ git diff
> diff --git a/tools/bpf/bpftool/btf_dumper.c b/tools/bpf/bpftool/btf_dumper.c
> index c64465094b92..045add07b721 100644
> --- a/tools/bpf/bpftool/btf_dumper.c
> +++ b/tools/bpf/bpftool/btf_dumper.c
> @@ -91,7 +91,8 @@ static void btf_dumper_int_bits(__u32 int_type, __u8 bit_offset,
>
> print_num = 0;
> memcpy(&print_num, data, bytes_to_copy);
> -#ifdef __BIG_ENDIAN_BITFIELD
> +#ifndef __LITTLE_ENDIAN_BITFIELD
> +#error "abc"
> left_shift_bits = bit_offset;
> #else
> left_shift_bits = 64 - bits_to_copy;
>
> $ make -C tools/bpf/bpftool/ CC=gcc-8
> make: Entering directory '/home/jkicinski/devel/linux/tools/bpf/bpftool'
> CC btf_dumper.o
> btf_dumper.c: In function ‘btf_dumper_int_bits’:
> btf_dumper.c:95:2: error: #error "abc"
> #error "abc"
> ^~~~~
> Makefile:96: recipe for target 'btf_dumper.o' failed
> make: *** [btf_dumper.o] Error 1
> make: Leaving directory '/home/jkicinski/devel/linux/tools/bpf/bpftool'
^ permalink raw reply
* Re: [PATCH bpf-next v4 2/3] bpf: btf: add btf print functionality
From: Okash Khawaja @ 2018-07-11 21:20 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Jakub Kicinski, Martin KaFai Lau, Alexei Starovoitov,
Yonghong Song, Quentin Monnet, David S. Miller, netdev,
kernel-team, linux-kernel
In-Reply-To: <8887ff9a-329e-3d89-8872-4bcc16c462e2@iogearbox.net>
On Wed, Jul 11, 2018 at 10:08:35PM +0200, Daniel Borkmann wrote:
> On 07/11/2018 09:10 PM, Jakub Kicinski wrote:
> > Thank you for all the changes made so far.
> >
> > On Tue, 10 Jul 2018 20:21:10 -0700, Okash Khawaja wrote:
> >> --- /dev/null
> >> +++ b/tools/bpf/bpftool/btf_dumper.c
> >> @@ -0,0 +1,248 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +/* Copyright (c) 2018 Facebook */
> >> +
> >> +#include <linux/btf.h>
> >> +#include <linux/err.h>
> >> +#include <stdio.h> /* for (FILE *) used by json_writer */
> >> +#include <linux/bitops.h>
> >> +#include <string.h>
> >> +#include <ctype.h>
> >
> > Again, please sort the headers the way I suggested. Otherwise as the
> > list of includes grows it's hard to know what's already there.
> >
> >> +#include "btf.h"
> >> +#include "json_writer.h"
> >> +#include "main.h"
> >> +
> >> +#define BITS_PER_BYTE_MASK (BITS_PER_BYTE - 1)
> >> +#define BITS_PER_BYTE_MASKED(bits) ((bits) & BITS_PER_BYTE_MASK)
> >> +#define BITS_ROUNDDOWN_BYTES(bits) ((bits) >> 3)
> >> +#define BITS_ROUNDUP_BYTES(bits) \
> >> + (BITS_ROUNDDOWN_BYTES(bits) + !!BITS_PER_BYTE_MASKED(bits))
> >> +
> >> +static int btf_dumper_do_type(const struct btf_dumper *d, __u32 type_id,
> >> + __u8 bit_offset, const void *data);
> >> +
> >> +static void btf_dumper_ptr(const void *data, json_writer_t *jw,
> >> + bool is_plain_text)
> >> +{
> >> + if (is_plain_text)
> >> + jsonw_printf(jw, "%p", *((unsigned long *)data));
> >> + else
> >> + jsonw_printf(jw, "%u", *((unsigned long *)data));
> >
> > Again, please drop the extraneous parens.
> >
> >> +}
> >> +
> >
> >> +static void btf_dumper_int_bits(__u32 int_type, __u8 bit_offset,
> >> + const void *data, json_writer_t *jw,
> >> + bool is_plain_text)
> >> +{
> >> + int left_shift_bits, right_shift_bits;
> >> + int nr_bits = BTF_INT_BITS(int_type);
> >> + int total_bits_offset;
> >> + int bytes_to_copy;
> >> + int bits_to_copy;
> >> + __u64 print_num;
> >> +
> >> + total_bits_offset = bit_offset + BTF_INT_OFFSET(int_type);
> >> + data += BITS_ROUNDDOWN_BYTES(total_bits_offset);
> >> + bit_offset = BITS_PER_BYTE_MASKED(total_bits_offset);
> >> + bits_to_copy = bit_offset + nr_bits;
> >> + bytes_to_copy = BITS_ROUNDUP_BYTES(bits_to_copy);
> >> +
> >> + print_num = 0;
> >> + memcpy(&print_num, data, bytes_to_copy);
> >> +#ifdef __BIG_ENDIAN_BITFIELD
> >> + left_shift_bits = bit_offset;
> >> +#else
> >> + left_shift_bits = 64 - bits_to_copy;
> >> +#endif
> >> + right_shift_bits = 64 - nr_bits;
> >
> > Please include <asm/byteorder.h> as I suggested to you previously.
> > This is dead code right now, look:
> >
> > $ git diff
> > diff --git a/tools/bpf/bpftool/btf_dumper.c b/tools/bpf/bpftool/btf_dumper.c
> > index c64465094b92..045add07b721 100644
> > --- a/tools/bpf/bpftool/btf_dumper.c
> > +++ b/tools/bpf/bpftool/btf_dumper.c
> > @@ -91,7 +91,8 @@ static void btf_dumper_int_bits(__u32 int_type, __u8 bit_offset,
> >
> > print_num = 0;
> > memcpy(&print_num, data, bytes_to_copy);
> > -#ifdef __BIG_ENDIAN_BITFIELD
> > +#ifndef __LITTLE_ENDIAN_BITFIELD
> > +#error "abc"
> > left_shift_bits = bit_offset;
> > #else
> > left_shift_bits = 64 - bits_to_copy;
> >
> > $ make -C tools/bpf/bpftool/ CC=gcc-8
> > make: Entering directory '/home/jkicinski/devel/linux/tools/bpf/bpftool'
> > CC btf_dumper.o
> > btf_dumper.c: In function ‘btf_dumper_int_bits’:
> > btf_dumper.c:95:2: error: #error "abc"
> > #error "abc"
> > ^~~~~
> > Makefile:96: recipe for target 'btf_dumper.o' failed
> > make: *** [btf_dumper.o] Error 1
> > make: Leaving directory '/home/jkicinski/devel/linux/tools/bpf/bpftool'
>
> You could also easily test this on s390x (big endian) through a LinuxONE
> test instance, this is how I usually test changes related to their JIT.
Thanks. I've been using MIPS qemu set up. This will definitely help.
>
> Thanks,
> Daniel
^ permalink raw reply
* Re: [PATCH net-next 2/2] net: phy: add phy_speed_down and phy_speed_up
From: Florian Fainelli @ 2018-07-11 21:33 UTC (permalink / raw)
To: Heiner Kallweit, Andrew Lunn; +Cc: David Miller, netdev@vger.kernel.org
In-Reply-To: <7dfcb4d5-2a0c-9244-53e4-564014b16b58@gmail.com>
On 07/11/2018 02:08 PM, Heiner Kallweit wrote:
> On 11.07.2018 22:55, Andrew Lunn wrote:
>>> +/**
>>> + * phy_speed_down - set speed to lowest speed supported by both link partners
>>> + * @phydev: the phy_device struct
>>> + * @sync: perform action synchronously
>>> + *
>>> + * Description: Typically used to save energy when waiting for a WoL packet
>>> + */
>>> +int phy_speed_down(struct phy_device *phydev, bool sync)
>>
>> This sync parameter needs some more thought. I'm not sure it is safe.
>>
>> How does a PHY trigger a WoL wake up? I guess some use the interrupt
>> pin. How does a PHY indicate auto-neg has completed? It triggers an
>> interrupt. So it seems like there is a danger here we suspend, and
>> then wake up 2 seconds later when auto-neg has completed.
>>
>> I'm not sure we can safely suspend until auto-neg has completed.
>>
>>> +/**
>>> + * phy_speed_up - (re)set advertised speeds to all supported speeds
>>> + * @phydev: the phy_device struct
>>> + * @sync: perform action synchronously
>>> + *
>>> + * Description: Used to revert the effect of phy_speed_down
>>> + */
>>> +int phy_speed_up(struct phy_device *phydev, bool sync)
>>
>> And here, i'm thinking the opposite. A MAC driver needs to be ready
>> for the PHY state to change at any time. So why do we need to wait?
>> Just let the normal mechanisms inform the MAC when the link is up.
>>
> I see your points, thanks for the feedback. In my case WoL triggers
> a PCI PME and the code works as expected, but I agree this may be
> different in other setups (external PHY).
>
> The sync parameter was inspired by following comment from Florian:
> "One thing that bothers me a bit is that this should ideally be
> offered as both blocking and non-blocking options"
> So let's see which comments he may have before preparing a v2.
What I had in mind is that you would be able to register a callback that
would tell you when auto-negotiation completes, and not register one if
you did not want to have that information.
As Andrew points out though, with PHY using interrupts, this might be a
bit challenging to do because you will get an interrupt about "something
has changed" and you would have to run the callback from the PHY state
machine to determine this was indeed a result of triggering
auto-negotiation. Maybe polling for auto-negotiation like you do here is
good enough.
One nit, you might have to check for those functions that the PHY did
have auto-negotiation enabled and was not forced.
--
Florian
^ permalink raw reply
* [PATCH net-next 0/2] docs: Fix failover build warnings
From: Tobin C. Harding @ 2018-07-11 21:42 UTC (permalink / raw)
To: David S. Miller
Cc: Tobin C. Harding, Sridhar Samudrala, linux-doc, netdev,
linux-kernel
Hi Dave,
This is my first patch set to net-next. Please shout loud and clear if
I've botched anything.
Recently failover and net_failover modules were added to the mainline.
Documentation was included in rst format but they were not added to the
toctree in `networking/index.rst`. Also building docs for net_failover
is currently emitting a few warnings.
Patch 1 adds failover and net_failover to the index toctree
Patch 2 fixes the build warnings for net_failover
I haven't been super active on netdev list so if there is some reason I
missed why these files are not in the index please do say so.
Has there been any discussion on preferred order for the toctree index
list? I just added them to the bottom of the list.
thanks,
Tobin.
Tobin C. Harding (2):
docs: networking: Add failover docs to index
docs: networking: Fix failover build warnings
Documentation/networking/index.rst | 2 +
Documentation/networking/net_failover.rst | 111 +++++++++++-----------
2 files changed, 59 insertions(+), 54 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH net-next 1/2] docs: networking: Add failover docs to index
From: Tobin C. Harding @ 2018-07-11 21:42 UTC (permalink / raw)
To: David S. Miller
Cc: Tobin C. Harding, Sridhar Samudrala, linux-doc, netdev,
linux-kernel
In-Reply-To: <20180711214250.19039-1-me@tobin.cc>
Currently we have rst format docs for the failover and net_failover
modules however these docs are not linked to within the index.
Add `failover` and `net_failover` to the networking documentation index.
Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
Documentation/networking/index.rst | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/networking/index.rst b/Documentation/networking/index.rst
index fec8588a588e..6123a7e9e1da 100644
--- a/Documentation/networking/index.rst
+++ b/Documentation/networking/index.rst
@@ -15,6 +15,8 @@ Contents:
kapi
z8530book
msg_zerocopy
+ failover
+ net_failover
.. only:: subproject
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 2/2] docs: networking: Fix failover build warnings
From: Tobin C. Harding @ 2018-07-11 21:42 UTC (permalink / raw)
To: David S. Miller
Cc: Tobin C. Harding, Sridhar Samudrala, linux-doc, netdev,
linux-kernel, Jonathan Corbet
In-Reply-To: <20180711214250.19039-1-me@tobin.cc>
Currently building the net_failover docs causes a bunch of warnings to
be emitted. These warnings are all related to indentation and correctly
highlight missing '::' (for code sections). It looks, from other rst
files in Documentation, that the first column should be indented 2
spaces.
Add '::' before code snippets and indent all snippets uniformly starting
with 2 spaces.
Cc: Jonathan Corbet <corbet@lwn.net>
Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
Cc'd Jon incase he wants to suggest a preferred level of indentation.
thanks,
Tobin.
Documentation/networking/net_failover.rst | 111 +++++++++++-----------
1 file changed, 57 insertions(+), 54 deletions(-)
diff --git a/Documentation/networking/net_failover.rst b/Documentation/networking/net_failover.rst
index 70ca2f5800c4..06c97dcb57ca 100644
--- a/Documentation/networking/net_failover.rst
+++ b/Documentation/networking/net_failover.rst
@@ -36,37 +36,39 @@ feature on the virtio-net interface and assign the same MAC address to both
virtio-net and VF interfaces.
Here is an example XML snippet that shows such configuration.
-
- <interface type='network'>
- <mac address='52:54:00:00:12:53'/>
- <source network='enp66s0f0_br'/>
- <target dev='tap01'/>
- <model type='virtio'/>
- <driver name='vhost' queues='4'/>
- <link state='down'/>
- <address type='pci' domain='0x0000' bus='0x00' slot='0x0a' function='0x0'/>
- </interface>
- <interface type='hostdev' managed='yes'>
- <mac address='52:54:00:00:12:53'/>
- <source>
- <address type='pci' domain='0x0000' bus='0x42' slot='0x02' function='0x5'/>
- </source>
- <address type='pci' domain='0x0000' bus='0x00' slot='0x0b' function='0x0'/>
- </interface>
+::
+
+ <interface type='network'>
+ <mac address='52:54:00:00:12:53'/>
+ <source network='enp66s0f0_br'/>
+ <target dev='tap01'/>
+ <model type='virtio'/>
+ <driver name='vhost' queues='4'/>
+ <link state='down'/>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x0a' function='0x0'/>
+ </interface>
+ <interface type='hostdev' managed='yes'>
+ <mac address='52:54:00:00:12:53'/>
+ <source>
+ <address type='pci' domain='0x0000' bus='0x42' slot='0x02' function='0x5'/>
+ </source>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x0b' function='0x0'/>
+ </interface>
Booting a VM with the above configuration will result in the following 3
netdevs created in the VM.
-
-4: ens10: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default qlen 1000
- link/ether 52:54:00:00:12:53 brd ff:ff:ff:ff:ff:ff
- inet 192.168.12.53/24 brd 192.168.12.255 scope global dynamic ens10
- valid_lft 42482sec preferred_lft 42482sec
- inet6 fe80::97d8:db2:8c10:b6d6/64 scope link
- valid_lft forever preferred_lft forever
-5: ens10nsby: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc fq_codel master ens10 state UP group default qlen 1000
- link/ether 52:54:00:00:12:53 brd ff:ff:ff:ff:ff:ff
-7: ens11: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq master ens10 state UP group default qlen 1000
- link/ether 52:54:00:00:12:53 brd ff:ff:ff:ff:ff:ff
+::
+
+ 4: ens10: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default qlen 1000
+ link/ether 52:54:00:00:12:53 brd ff:ff:ff:ff:ff:ff
+ inet 192.168.12.53/24 brd 192.168.12.255 scope global dynamic ens10
+ valid_lft 42482sec preferred_lft 42482sec
+ inet6 fe80::97d8:db2:8c10:b6d6/64 scope link
+ valid_lft forever preferred_lft forever
+ 5: ens10nsby: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc fq_codel master ens10 state UP group default qlen 1000
+ link/ether 52:54:00:00:12:53 brd ff:ff:ff:ff:ff:ff
+ 7: ens11: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq master ens10 state UP group default qlen 1000
+ link/ether 52:54:00:00:12:53 brd ff:ff:ff:ff:ff:ff
ens10 is the 'failover' master netdev, ens10nsby and ens11 are the slave
'standby' and 'primary' netdevs respectively.
@@ -80,37 +82,38 @@ the paravirtual datapath when the VF is unplugged.
Here is a sample script that shows the steps to initiate live migration on
the source hypervisor.
+::
-# cat vf_xml
-<interface type='hostdev' managed='yes'>
- <mac address='52:54:00:00:12:53'/>
- <source>
- <address type='pci' domain='0x0000' bus='0x42' slot='0x02' function='0x5'/>
- </source>
- <address type='pci' domain='0x0000' bus='0x00' slot='0x0b' function='0x0'/>
-</interface>
+ # cat vf_xml
+ <interface type='hostdev' managed='yes'>
+ <mac address='52:54:00:00:12:53'/>
+ <source>
+ <address type='pci' domain='0x0000' bus='0x42' slot='0x02' function='0x5'/>
+ </source>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x0b' function='0x0'/>
+ </interface>
-# Source Hypervisor
-#!/bin/bash
+ # Source Hypervisor
+ #!/bin/bash
-DOMAIN=fedora27-tap01
-PF=enp66s0f0
-VF_NUM=5
-TAP_IF=tap01
-VF_XML=
+ DOMAIN=fedora27-tap01
+ PF=enp66s0f0
+ VF_NUM=5
+ TAP_IF=tap01
+ VF_XML=
-MAC=52:54:00:00:12:53
-ZERO_MAC=00:00:00:00:00:00
+ MAC=52:54:00:00:12:53
+ ZERO_MAC=00:00:00:00:00:00
-virsh domif-setlink $DOMAIN $TAP_IF up
-bridge fdb del $MAC dev $PF master
-virsh detach-device $DOMAIN $VF_XML
-ip link set $PF vf $VF_NUM mac $ZERO_MAC
+ virsh domif-setlink $DOMAIN $TAP_IF up
+ bridge fdb del $MAC dev $PF master
+ virsh detach-device $DOMAIN $VF_XML
+ ip link set $PF vf $VF_NUM mac $ZERO_MAC
-virsh migrate --live $DOMAIN qemu+ssh://$REMOTE_HOST/system
+ virsh migrate --live $DOMAIN qemu+ssh://$REMOTE_HOST/system
-# Destination Hypervisor
-#!/bin/bash
+ # Destination Hypervisor
+ #!/bin/bash
-virsh attach-device $DOMAIN $VF_XML
-virsh domif-setlink $DOMAIN $TAP_IF down
+ virsh attach-device $DOMAIN $VF_XML
+ virsh domif-setlink $DOMAIN $TAP_IF down
--
2.17.1
^ permalink raw reply related
* Re: [PATCH net-next 2/2] net: phy: add phy_speed_down and phy_speed_up
From: Heiner Kallweit @ 2018-07-11 21:59 UTC (permalink / raw)
To: Florian Fainelli, Andrew Lunn; +Cc: David Miller, netdev@vger.kernel.org
In-Reply-To: <28ea392b-6d3a-0efe-a0ed-ebe82fe14099@gmail.com>
On 11.07.2018 23:33, Florian Fainelli wrote:
>
>
> On 07/11/2018 02:08 PM, Heiner Kallweit wrote:
>> On 11.07.2018 22:55, Andrew Lunn wrote:
>>>> +/**
>>>> + * phy_speed_down - set speed to lowest speed supported by both link partners
>>>> + * @phydev: the phy_device struct
>>>> + * @sync: perform action synchronously
>>>> + *
>>>> + * Description: Typically used to save energy when waiting for a WoL packet
>>>> + */
>>>> +int phy_speed_down(struct phy_device *phydev, bool sync)
>>>
>>> This sync parameter needs some more thought. I'm not sure it is safe.
>>>
>>> How does a PHY trigger a WoL wake up? I guess some use the interrupt
>>> pin. How does a PHY indicate auto-neg has completed? It triggers an
>>> interrupt. So it seems like there is a danger here we suspend, and
>>> then wake up 2 seconds later when auto-neg has completed.
>>>
>>> I'm not sure we can safely suspend until auto-neg has completed.
>>>
>>>> +/**
>>>> + * phy_speed_up - (re)set advertised speeds to all supported speeds
>>>> + * @phydev: the phy_device struct
>>>> + * @sync: perform action synchronously
>>>> + *
>>>> + * Description: Used to revert the effect of phy_speed_down
>>>> + */
>>>> +int phy_speed_up(struct phy_device *phydev, bool sync)
>>>
>>> And here, i'm thinking the opposite. A MAC driver needs to be ready
>>> for the PHY state to change at any time. So why do we need to wait?
>>> Just let the normal mechanisms inform the MAC when the link is up.
>>>
>> I see your points, thanks for the feedback. In my case WoL triggers
>> a PCI PME and the code works as expected, but I agree this may be
>> different in other setups (external PHY).
>>
>> The sync parameter was inspired by following comment from Florian:
>> "One thing that bothers me a bit is that this should ideally be
>> offered as both blocking and non-blocking options"
>> So let's see which comments he may have before preparing a v2.
>
> What I had in mind is that you would be able to register a callback that
> would tell you when auto-negotiation completes, and not register one if
> you did not want to have that information.
>
> As Andrew points out though, with PHY using interrupts, this might be a
> bit challenging to do because you will get an interrupt about "something
> has changed" and you would have to run the callback from the PHY state
> machine to determine this was indeed a result of triggering
> auto-negotiation. Maybe polling for auto-negotiation like you do here is
> good enough.
>
OK, then I would poll for autoneg finished in phy_speed_down and
remove the polling option from phy_speed_up. I will do some tests
with this before submitting a v2.
> One nit, you might have to check for those functions that the PHY did
> have auto-negotiation enabled and was not forced.
>
This I'm doing already, or do you mean something different?
^ permalink raw reply
* Re: [PATCH bpf v2 1/1] bpf: btf: Fix bitfield extraction for big endian
From: Daniel Borkmann @ 2018-07-11 22:12 UTC (permalink / raw)
To: Martin KaFai Lau, Okash Khawaja
Cc: Alexei Starovoitov, Yonghong Song, Jakub Kicinski,
David S. Miller, netdev, kernel-team, linux-kernel
In-Reply-To: <20180710234528.2kj2kv7gjmigvvgp@kafai-mbp.dhcp.thefacebook.com>
On 07/11/2018 01:46 AM, Martin KaFai Lau wrote:
> On Tue, Jul 10, 2018 at 02:33:07PM -0700, Okash Khawaja wrote:
>> When extracting bitfield from a number, btf_int_bits_seq_show() builds
>> a mask and accesses least significant byte of the number in a way
>> specific to little-endian. This patch fixes that by checking endianness
>> of the machine and then shifting left and right the unneeded bits.
>>
>> Thanks to Martin Lau for the help in navigating potential pitfalls when
>> dealing with endianess and for the final solution.
>>
>> Fixes: b00b8daec828 ("bpf: btf: Add pretty print capability for data with BTF type info")
>> Signed-off-by: Okash Khawaja <osk@fb.com>
> Acked-by: Martin KaFai Lau <kafai@fb.com>
Applied to bpf, thanks Okash!
^ permalink raw reply
* [BUG] bonded interfaces drop bpdu (stp) frames
From: Michal Soltys @ 2018-07-11 22:23 UTC (permalink / raw)
To: netdev
Hi,
As weird as that sounds, this is what I observed today after bumping
kernel version. I have a setup where 2 bonds are attached to linux
bridge and physically are connected to two switches doing MSTP (and
linux bridge is just passing them).
Initially I suspected some changes related to bridge code - but quick
peek at the code showed nothing suspicious - and the part of it that
explicitly passes stp frames if stp is not enabled has seen little
changes (e.g. per-port group_fwd_mask added recently). Furthermore - if
regular non-bonded interfaces are attached everything works fine.
Just to be sure I detached the bond (802.3ad mode) and checked it with
simple tcpdump (ether proto \\stp) - and indeed no hello packets were
there (with them being present just fine on active enslaved interface,
or on the bond device in earlier kernels).
If time permits I'll bisect tommorow to pinpoint the commit, but from
quick todays test - 4.9.x is working fine, while 4.16.16 (tested on
debian) and 4.17.3 (tested on archlinux) are failing.
Unless this is already a known issue (or you have any suggestions what
could be responsible).
^ permalink raw reply
* [BUG net-next] BUG triggered with GRO SKB list_head changes
From: Tyler Hicks @ 2018-07-11 22:39 UTC (permalink / raw)
To: David Miller; +Cc: netdev
[-- Attachment #1: Type: text/plain, Size: 5462 bytes --]
Starting with the following net-next commit, I see a BUG when starting a
LXD container inside of a KVM guest using virtio-net:
d4546c2509b1 net: Convert GRO SKB handling to list_head.
Here's what the kernel spits out:
kernel BUG at /var/scm/kernel/linux/include/linux/skbuff.h:2080!
invalid opcode: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC PTI
CPU: 0 PID: 1362 Comm: libvirtd Not tainted 4.18.0-rc2+ #69
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Ubuntu-1.8.2-1ubuntu1 04/01/2014
RIP: 0010:skb_pull+0x36/0x40
Code: c6 77 24 29 f0 3b 87 84 00 00 00 89 87 80 00 00 00 72 17 89 f6 48 89 f0 48 03 87 d8 00 00 00 48 89 87 d8 00 00 00 c3 31 c0 c3 <0f> 0b 0f 1f 84 00 00 00
00 00 0f 1f 44 00 00 39 b7 80 00 00 00 76
RSP: 0000:ffff96737f6039f0 EFLAGS: 00010297
RAX: 000000009c66e2f2 RBX: 0000000000000000 RCX: 0000000000000501
RDX: 0000000000000001 RSI: 000000000000000e RDI: ffff96737f7e3938
RBP: ffff967379f40020 R08: 0000000000000000 R09: 0000000000000000
R10: ffff96737f603988 R11: ffffffffc0461335 R12: ffff967379f409e0
R13: ffff96737f7e3938 R14: 0000000000000000 R15: ffff967379e96ac0
FS: 00007fc96087e640(0000) GS:ffff96737f600000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007fc913608aa0 CR3: 000000005dacc001 CR4: 00000000001606f0
Call Trace:
<IRQ>
br_dev_xmit+0xe1/0x3d0 [bridge]
dev_hard_start_xmit+0xbc/0x3b0
__dev_queue_xmit+0xb98/0xc30
ip_finish_output2+0x3e5/0x670
? ip_output+0x7f/0x250
ip_output+0x7f/0x250
? ip_fragment.constprop.5+0x80/0x80
ip_forward+0x3e2/0x650
? ipv4_frags_init_net+0x130/0x130
ip_rcv+0x2be/0x500
? ip_local_deliver_finish+0x3b0/0x3b0
__netif_receive_skb_core+0x6a8/0xb30
? lock_acquire+0xab/0x200
? netif_receive_skb_internal+0x2a/0x380
netif_receive_skb_internal+0x73/0x380
? napi_gro_complete+0xcf/0x1b0
dev_gro_receive+0x374/0x730
napi_gro_receive+0x4f/0x1d0
receive_buf+0x4b6/0x1930 [virtio_net]
? detach_buf+0x69/0x120 [virtio_ring]
virtnet_poll+0x122/0x2e0 [virtio_net]
net_rx_action+0x207/0x450
__do_softirq+0x149/0x4ea
irq_exit+0xbf/0xd0
do_IRQ+0x6c/0x130
common_interrupt+0xf/0xf
</IRQ>
RIP: 0010:__radix_tree_lookup+0x28/0xe0
Code: 00 00 53 49 89 ca 41 bb 40 00 00 00 4c 8b 47 50 4c 89 c0 83 e0 03 48 83 f8 01 0f 85 a8 00 00 00 4c 89 c0 48 83 e0 fe 0f b6 08 <4c> 89 d8 48 d3 e0 48 83
e8 01 48 39 c6 76 11 e9 9f 00 00 00 4c 89
RSP: 0000:ffffae150048fcc0 EFLAGS: 00000282 ORIG_RAX: ffffffffffffffd9
RAX: ffff96735d2ef908 RBX: 000000000000001f RCX: 0000000000000006
RDX: 0000000000000000 RSI: 00000000000002e2 RDI: ffff96735d10b788
RBP: 00000000000002e2 R08: ffff96735d2ef909 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000040 R12: 000000000000001f
R13: ffffec01c15f3a80 R14: 000000000000001f R15: ffffae150048fd18
__do_page_cache_readahead+0x11f/0x2e0
filemap_fault+0x408/0x660
ext4_filemap_fault+0x2f/0x40
__do_fault+0x1f/0xd0
__handle_mm_fault+0x915/0xfa0
handle_mm_fault+0x1c2/0x390
__do_page_fault+0x2f6/0x580
? async_page_fault+0x5/0x20
async_page_fault+0x1b/0x20
RIP: 0033:0x7fc913608aa0
Code: Bad RIP value.
RSP: 002b:00007ffcfa9c7f08 EFLAGS: 00010206
RAX: 0000000000000000 RBX: 0000000000000003 RCX: 0000000000000080
RDX: 0000000000000006 RSI: 00007fc913a74bf8 RDI: 00007fc913df9720
RBP: 0000000000000001 R08: 000055df45795700 R09: 0000000000000000
R10: 000055df4574c010 R11: 0000000000000001 R12: 00007ffcfa9c8c38
R13: 00007ffcfa9c8c48 R14: 00007fc913dc3d70 R15: 000055df4578ab30
Modules linked in: veth ebtable_filter ebtables ipt_MASQUERADE xt_CHECKSUM xt_comment xt_tcpudp iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack libcrc32c iptable_mangle iptable_filter bpfilter bridge stp llc fuse kvm_intel kvm irqbypass 9pnet_virtio 9pnet virtio_balloon ib_iser rdma_cm configfs iw_cm ib_cm ib_core iscsi_tcp libiscsi_tcp libiscsi scsi_transport_iscsi ip_tables x_tables virtio_net net_failover virtio_blk failover crc32_pclmul crc32c_intel pcbc aesni_intel aes_x86_64 crypto_simd cryptd glue_helper virtio_pci psmouse virtio_ring virtio
I'm not very familiar with the GRO or IP fragmentation code but I was
able to identify that this change "fixes" the issue:
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 7ccc601b55d9..a5cea572a7f1 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -666,6 +666,7 @@ struct sk_buff {
/* These two members must be first. */
struct sk_buff *next;
struct sk_buff *prev;
+ struct list_head list;
union {
struct net_device *dev;
@@ -678,7 +679,6 @@ struct sk_buff {
};
};
struct rb_node rbnode; /* used in netem & tcp stack */
- struct list_head list;
};
struct sock *sk;
That's not the correct fix, as we wouldn't want to waste space with two
list implementations always being around, but I think it shows that
perhaps there is something in the call stack attempting to use both the
list_head list and the ip_defrag_offset at the same time and
unintentionally trouncing over the other member in the union.
I wish I had a proper fix but I suspect that someone more familiar with
this code will spot the issue quickly. I didn't see anything incorrect
in the list manipulations in the offending commit so some deeper
knowledge of the network stack is needed.
Tyler
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply related
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