* Re: [PATCH net-next v2 0/5] bpf: BPF for lightweight tunnel encapsulation
From: Tom Herbert @ 2016-11-01 16:17 UTC (permalink / raw)
To: Thomas Graf
Cc: David S. Miller, Alexei Starovoitov, Daniel Borkmann, roopa,
Linux Kernel Network Developers
In-Reply-To: <cover.1477959702.git.tgraf@suug.ch>
On Mon, Oct 31, 2016 at 5:37 PM, Thomas Graf <tgraf@suug.ch> wrote:
> {Open question:
> Tom brought up the question on whether it is safe to modify the packet
> in artbirary ways before dst_output(). This is the equivalent to a raw
> socket injecting illegal headers. This v2 currently assumes that
> dst_output() is ready to accept invalid header values. This needs to be
> verified and if not the case, then raw sockets or dst_output() handlers
> must be fixed as well. Another option is to mark lwtunnel_output() as
> read-only for now.}
>
The question might not be so much about illegal headers but whether
fields in the skbuff related to the packet contents are kept correct.
We have protocol, header offsets, offsets for inner protocols also,
encapsulation settings, checksum status, checksum offset, checksum
complete value, vlan information. Any or all of which I believe could
be turned into being incorrect if we allow the packet to be
arbitrarily modified by BPF. This problem is different than raw
sockets because LWT operates in the middle of the stack, the skbuff
has already been set up which such things.
> This series implements BPF program invocation from dst entries via the
> lightweight tunnels infrastructure. The BPF program can be attached to
> lwtunnel_input(), lwtunnel_output() or lwtunnel_xmit() and sees an L3
> skb as context. input is read-only, output can write, xmit can write,
> push headers, and redirect.
>
> Motiviation for this work:
> - Restricting outgoing routes beyond what the route tuple supports
> - Per route accounting byond realms
> - Fast attachment of L2 headers where header does not require resolving
> L2 addresses
> - ILA like uses cases where L3 addresses are resolved and then routed
> in an async manner
> - Fast encapsulation + redirect. For now limited to use cases where not
> setting inner and outer offset/protocol is OK.
>
Is checksum offload supported? By default, at least for Linux, we
offload the outer UDP checksum in VXLAN and the other UDP
encapsulations for performance.
Tom
> A couple of samples on how to use it can be found in patch 04.
>
> v1 -> v2:
> - Added new BPF_LWT_REROUTE return code for program to indicate
> that new route lookup should be performed. Suggested by Tom.
> - New sample to illustrate rerouting
> - New patch 05: Recursion limit for lwtunnel_output for the case
> when user creates circular dst redirection. Also resolves the
> issue for ILA.
> - Fix to ensure headroom for potential future L2 header is still
> guaranteed
>
> Thomas Graf (5):
> route: Set orig_output when redirecting to lwt on locally generated
> traffic
> route: Set lwtstate for local traffic and cached input dsts
> bpf: BPF for lightweight tunnel encapsulation
> bpf: Add samples for LWT-BPF
> lwtunnel: Limit number of recursions on output to 5
>
> include/linux/filter.h | 2 +-
> include/uapi/linux/bpf.h | 37 +++-
> include/uapi/linux/lwtunnel.h | 21 ++
> kernel/bpf/verifier.c | 16 +-
> net/Kconfig | 1 +
> net/core/Makefile | 2 +-
> net/core/filter.c | 148 ++++++++++++-
> net/core/lwt_bpf.c | 504 ++++++++++++++++++++++++++++++++++++++++++
> net/core/lwtunnel.c | 15 +-
> net/ipv4/route.c | 37 +++-
> samples/bpf/bpf_helpers.h | 4 +
> samples/bpf/lwt_bpf.c | 235 ++++++++++++++++++++
> samples/bpf/test_lwt_bpf.sh | 370 +++++++++++++++++++++++++++++++
> 13 files changed, 1373 insertions(+), 19 deletions(-)
> create mode 100644 net/core/lwt_bpf.c
> create mode 100644 samples/bpf/lwt_bpf.c
> create mode 100755 samples/bpf/test_lwt_bpf.sh
>
> --
> 2.7.4
>
^ permalink raw reply
* Re: [PATCH v2] unix: escape all null bytes in abstract unix domain socket
From: David Miller @ 2016-11-01 16:15 UTC (permalink / raw)
To: iboukris; +Cc: stephen, netdev, linux-kernel
In-Reply-To: <1477960895-1062-1-git-send-email-iboukris@gmail.com>
From: Isaac Boukris <iboukris@gmail.com>
Date: Tue, 1 Nov 2016 02:41:35 +0200
> Abstract unix domain socket may embed null characters,
> these should be translated to '@' when printed out to
> proc the same way the null prefix is currently being
> translated.
>
> This helps for tools such as netstat, lsof and the proc
> based implementation in ss to show all the significant
> bytes of the name (instead of getting cut at the first
> null occurrence).
>
> Signed-off-by: Isaac Boukris <iboukris@gmail.com>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH] net: qcom/emac: use correct value for SGMII_LN_UCDR_SO_GAIN_MODE0
From: David Miller @ 2016-11-01 16:14 UTC (permalink / raw)
To: timur; +Cc: netdev
In-Reply-To: <1477955922-18276-1-git-send-email-timur@codeaurora.org>
From: Timur Tabi <timur@codeaurora.org>
Date: Mon, 31 Oct 2016 18:18:42 -0500
> The documentation says that SGMII_LN_UCDR_SO_GAIN_MODE0 should be
> set to 0, not 6, on the Qualcomm Technologies QDF2432.
>
> Signed-off-by: Timur Tabi <timur@codeaurora.org>
Applied.
^ permalink raw reply
* Re: [PATCH net-next v2] genetlink: fix error return code in genl_register_family()
From: David Miller @ 2016-11-01 16:13 UTC (permalink / raw)
To: weiyj.lk
Cc: stephen, tom, fw, johannes.berg, pshelar, tycho.andersen,
matti.vaittinen, weiyongjun1, netdev
In-Reply-To: <1478011552-24957-1-git-send-email-weiyj.lk@gmail.com>
From: Wei Yongjun <weiyj.lk@gmail.com>
Date: Tue, 1 Nov 2016 14:45:52 +0000
> From: Wei Yongjun <weiyongjun1@huawei.com>
>
> Fix to return a negative error code from the idr_alloc() error handling
> case instead of 0, as done elsewhere in this function.
>
> Also fix the return value check of idr_alloc() since idr_alloc return
> negative errors on failure, not zero.
>
> Fixes: 2ae0f17df1cd ("genetlink: use idr to track families")
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
> ---
> v1 -> v2: fix the return value check and return idr_alloc's err code
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next 0/2] drivers: net: xgene: Fix coalescing bugs
From: David Miller @ 2016-11-01 16:05 UTC (permalink / raw)
To: isubramanian; +Cc: netdev, linux-arm-kernel, patches
In-Reply-To: <1477954827-9951-1-git-send-email-isubramanian@apm.com>
From: Iyappan Subramanian <isubramanian@apm.com>
Date: Mon, 31 Oct 2016 16:00:25 -0700
> This patch set fixes the following,
>
> 1. Since ethernet v1 hardware has a bug related to coalescing,
> disabling this feature
> 2. Fixing ethernet v2 hardware, interrupt trigger region
> id to 2, to kickoff coalescing
>
> Signed-off-by: Iyappan Subramanian <isubramanian@apm.com>
Series applied, thanks.
^ permalink raw reply
* Re: [PATCH v3 net-next] lan78xx: Use irq_domain for phy interrupt from USB Int. EP
From: David Miller @ 2016-11-01 16:03 UTC (permalink / raw)
To: Woojung.Huh; +Cc: netdev, f.fainelli, andrew, UNGLinuxDriver
In-Reply-To: <9235D6609DB808459E95D78E17F2E43D4095CACB@CHN-SV-EXMX02.mchp-main.com>
From: <Woojung.Huh@microchip.com>
Date: Mon, 31 Oct 2016 21:44:37 +0000
> @@ -2668,6 +2821,13 @@ static int lan78xx_bind(struct lan78xx_net *dev, struct usb_interface *intf)
>
> dev->net->hw_features = dev->net->features;
>
> + ret = lan78xx_setup_irq_domain(dev);
> + if (ret < 0) {
> + netdev_warn(dev->net,
> + "lan78xx_setup_irq_domain() failed : %d", ret);
> + return ret;
> + }
> +
This error path leaks the memory allocated at the beginning of this function.
That would normally be freed up by lan78xx_unbind() but the caller will not
invoke that if lan78xx_bind() fails so you have to take care of the kfree
here.
^ permalink raw reply
* Re: [PATCH v2 2/6] net: phy: broadcom: Add BCM54810 PHY entry
From: Jon Mason @ 2016-11-01 15:59 UTC (permalink / raw)
To: Andrew Lunn
Cc: Mark Rutland, devicetree, Florian Fainelli, netdev, linux-kernel,
Rob Herring, bcm-kernel-feedback-list, rafal, David Miller,
linux-arm-kernel
In-Reply-To: <20161029081839.GA32579@lunn.ch>
On Sat, Oct 29, 2016 at 10:18:39AM +0200, Andrew Lunn wrote:
> On Fri, Oct 28, 2016 at 04:56:55PM -0400, Jon Mason wrote:
> > The BCM54810 PHY requires some semi-unique configuration, which results
> > in some additional configuration in addition to the standard config.
> > Also, some users of the BCM54810 require the PHY lanes to be swapped.
> > Since there is no way to detect this, add a device tree query to see if
> > it is applicable.
> >
> > Inspired-by: Vikas Soni <vsoni@broadcom.com>
> > Signed-off-by: Jon Mason <jon.mason@broadcom.com>
> > ---
> > drivers/net/phy/Kconfig | 2 +-
> > drivers/net/phy/broadcom.c | 58 +++++++++++++++++++++++++++++++++++++++++++++-
> > include/linux/brcmphy.h | 10 ++++++++
>
> Hi Jon
>
> The binding documentation is missing.
>
> > + if (of_property_read_bool(np, "brcm,enet-phy-lane-swap")) {
> > + /* Lane Swap - Undocumented register...magic! */
> > + ret = bcm_phy_write_exp(phydev, MII_BCM54XX_EXP_SEL_ER + 0x9,
> > + 0x11B);
> > + if (ret < 0)
> > + return ret;
> > + }
> > +
>
> I wounder if this property could be made generic? What exactly are you
> swapping? Rx and Tx lanes? Maybe we should add it to phy.txt?
Are you envisioning adding a DT check (similar to the
of_property_read_bool above, only with a more generic string) in
phy_device_create(), which will then set a PHY device flag? This flag
would then be checked for in the PHY driver and the appropriate action
taken (in this case the bcm_phy_write_exp above).
If so, I cam completely fine doing this. I think the only caveat
would be that this would be creating a generic interface for only 1
user. If you envision this being used by others, then disregard my
concern.
Thanks,
Jon
>
> Andrew
^ permalink raw reply
* Re: [PATCH net-next] sctp: clean up sctp_packet_transmit
From: David Miller @ 2016-11-01 15:58 UTC (permalink / raw)
To: lucien.xin; +Cc: netdev, linux-sctp, marcelo.leitner, nhorman, vyasevich
In-Reply-To: <407056b761e77ecbd43603a17099bb9309a9a8ac.1477932581.git.lucien.xin@gmail.com>
From: Xin Long <lucien.xin@gmail.com>
Date: Tue, 1 Nov 2016 00:49:41 +0800
> After adding sctp gso, sctp_packet_transmit is a quite big function now.
>
> This patch is to extract the codes for packing packet to sctp_packet_pack
> from sctp_packet_transmit, and add some comments, simplify the err path by
> freeing auth chunk when freeing packet chunk_list in out path and freeing
> head skb early if it fails to pack packet.
>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
SCTP experts, please review.
^ permalink raw reply
* Re: [net-next PATCH 0/7] stmmac: dwmac-sti refactor+cleanup
From: David Miller @ 2016-11-01 15:56 UTC (permalink / raw)
To: manabian; +Cc: peppe.cavallaro, alexandre.torgue, netdev
In-Reply-To: <20161031.154726.460941249322981922.davem@davemloft.net>
From: David Miller <davem@davemloft.net>
Date: Mon, 31 Oct 2016 15:47:26 -0400 (EDT)
> From: Joachim Eastwood <manabian@gmail.com>
> Date: Sun, 30 Oct 2016 21:05:00 +0100
>
>> This patch set aims to remove the init/exit callbacks from the
>> dwmac-sti driver and instead use standard PM callbacks. Doing this
>> will also allow us to cleanup the driver.
>>
>> Eventually the init/exit callbacks will be deprecated and removed
>> from all drivers dwmac-* except for dwmac-generic. Drivers will be
>> refactored to use standard PM and remove callbacks.
>>
>> Note that this patch set has only been test compiled and no functional
>> change is intended.
>
> I would really like to see some review and testing before applying this
> series.
Ping?
^ permalink raw reply
* Re: [PATCH net-next] net: Enable support for VRF with ipv4 multicast
From: David Miller @ 2016-11-01 15:54 UTC (permalink / raw)
To: dsa; +Cc: netdev
In-Reply-To: <1477954440-12339-1-git-send-email-dsa@cumulusnetworks.com>
From: David Ahern <dsa@cumulusnetworks.com>
Date: Mon, 31 Oct 2016 15:54:00 -0700
> Enable support for IPv4 multicast:
> - similar to unicast the flow struct is updated to L3 master device
> if relevant prior to calling fib_rules_lookup. The table id is saved
> to the lookup arg so the rule action for ipmr can return the table
> associated with the device.
>
> - ip_mr_forward needs to check for master device mismatch as well
> since the skb->dev is set to it
>
> - allow multicast address on VRF device for Rx by checking for the
> daddr in the VRF device as well as the original ingress device
>
> - on Tx need to drop to __mkroute_output when FIB lookup fails for
> multicast destination address.
>
> - if CONFIG_IP_MROUTE_MULTIPLE_TABLES is enabled VRF driver creates
> IPMR FIB rules on first device create similar to FIB rules. In
> addition the VRF driver does not divert IPv4 multicast packets:
> it breaks on Tx since the fib lookup fails on the mcast address.
>
> With this patch, ipmr forwarding and local rx/tx work.
>
> Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
Applied, thanks David.
^ permalink raw reply
* Re: [PATCH net-next v2 00/16] tipc: socket layer improvements
From: David Miller @ 2016-11-01 15:53 UTC (permalink / raw)
To: parthasarathy.bhuvaragan; +Cc: jon.maloy, netdev, tipc-discussion
In-Reply-To: <1478005369-17239-1-git-send-email-parthasarathy.bhuvaragan@ericsson.com>
From: Parthasarathy Bhuvaragan <parthasarathy.bhuvaragan@ericsson.com>
Date: Tue, 1 Nov 2016 14:02:33 +0100
> The following issues with the current socket layer hinders socket
> diagnostics implementation, which led to this patch series.
Series applied, thank you.
------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
^ permalink raw reply
* Re: [PATCH net-next 0/3] tools lib bpf: Synchronize implementations
From: David Miller @ 2016-11-01 15:48 UTC (permalink / raw)
To: joe; +Cc: netdev, wangnan0, ast, daniel
In-Reply-To: <20161031183917.9938-1-joe@ovn.org>
From: Joe Stringer <joe@ovn.org>
Date: Mon, 31 Oct 2016 11:39:14 -0700
> Update tools/lib/bpf to provide more functionality and improve interoperation
> with other tools that generate and use eBPF code.
>
> The kernel uapi headers are a bit newer than the version in the tools/
> directory; synchronize those.
>
> samples/bpf/libbpf* has a bit more functionality than tools/lib/bpf, so extend
> tools/lib/bpf/bpf* with these functions to bring them into parity.
>
> tools/lib/bpf cannot read ELFs that tc can read, and vice versa. Update the
> map definition to be the same as in tc so the ELFs may be interchangeable
> (at least for now; I don't have a long-term plan in mind to ensure this always
> works).
Alexei and Daniel, please review.
^ permalink raw reply
* [PATCH net v3 2/2] ip6_udp_tunnel: remove unused IPCB related codes
From: Eli Cooper @ 2016-11-01 15:45 UTC (permalink / raw)
To: netdev, David S . Miller; +Cc: Shmulik Ladkani, Tom Herbert
In-Reply-To: <20161101154513.30347-1-elicooper@gmx.com>
Some IPCB fields are currently set in udp_tunnel6_xmit_skb(), which are
never used before it reaches ip6tunnel_xmit(), and past that point the
control buffer is no longer interpreted as IPCB.
This clears these unused IPCB related codes. Currently there is no skb
scrubbing in ip6_udp_tunnel, otherwise IPCB(skb)->opt might need to be
cleared for IPv4 packets, as shown in 5146d1f1511
("tunnel: Clear IPCB(skb)->opt before dst_link_failure called").
Signed-off-by: Eli Cooper <elicooper@gmx.com>
---
net/ipv6/ip6_udp_tunnel.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/net/ipv6/ip6_udp_tunnel.c b/net/ipv6/ip6_udp_tunnel.c
index a752052..b283f29 100644
--- a/net/ipv6/ip6_udp_tunnel.c
+++ b/net/ipv6/ip6_udp_tunnel.c
@@ -88,9 +88,6 @@ int udp_tunnel6_xmit_skb(struct dst_entry *dst, struct sock *sk,
uh->len = htons(skb->len);
- memset(&(IPCB(skb)->opt), 0, sizeof(IPCB(skb)->opt));
- IPCB(skb)->flags &= ~(IPSKB_XFRM_TUNNEL_SIZE | IPSKB_XFRM_TRANSFORMED
- | IPSKB_REROUTED);
skb_dst_set(skb, dst);
udp6_set_csum(nocheck, skb, saddr, daddr, skb->len);
--
2.10.1
^ permalink raw reply related
* [PATCH net v3 1/2] ip6_tunnel: Clear IP6CB in ip6tunnel_xmit()
From: Eli Cooper @ 2016-11-01 15:45 UTC (permalink / raw)
To: netdev, David S . Miller; +Cc: Shmulik Ladkani, Tom Herbert
skb->cb may contain data from previous layers. In the observed scenario,
the garbage data were misinterpreted as IP6CB(skb)->frag_max_size, so
that small packets sent through the tunnel are mistakenly fragmented.
This patch unconditionally clears the control buffer in ip6tunnel_xmit(),
which affects ip6_tunnel, ip6_udp_tunnel and ip6_gre. Currently none of
these tunnels set IP6CB(skb)->flags, otherwise it needs to be done earlier.
Cc: stable@vger.kernel.org
Signed-off-by: Eli Cooper <elicooper@gmx.com>
---
v3: moves to ip6tunnel_xmit() and clears IP6CB unconditionally
v2: clears the whole IP6CB altogether and does it after encapsulation
include/net/ip6_tunnel.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/net/ip6_tunnel.h b/include/net/ip6_tunnel.h
index 20ed969..1b1cf33 100644
--- a/include/net/ip6_tunnel.h
+++ b/include/net/ip6_tunnel.h
@@ -146,6 +146,7 @@ static inline void ip6tunnel_xmit(struct sock *sk, struct sk_buff *skb,
{
int pkt_len, err;
+ memset(skb->cb, 0, sizeof(struct inet6_skb_parm));
pkt_len = skb->len - skb_inner_network_offset(skb);
err = ip6_local_out(dev_net(skb_dst(skb)->dev), sk, skb);
if (unlikely(net_xmit_eval(err)))
--
2.10.1
^ permalink raw reply related
* Re: [PATCH net-next] mlxsw: switchib: Remove unused including <generated/utsrelease.h>
From: Jiri Pirko @ 2016-11-01 15:44 UTC (permalink / raw)
To: Wei Yongjun; +Cc: Jiri Pirko, Ido Schimmel, Wei Yongjun, netdev
In-Reply-To: <1478013248-9409-1-git-send-email-weiyj.lk@gmail.com>
Tue, Nov 01, 2016 at 04:14:08PM CET, weiyj.lk@gmail.com wrote:
>From: Wei Yongjun <weiyongjun1@huawei.com>
>
>Remove including <generated/utsrelease.h> that don't need it.
>
>Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
>---
> drivers/net/ethernet/mellanox/mlxsw/switchib.c | 1 -
> 1 file changed, 1 deletion(-)
>
>diff --git a/drivers/net/ethernet/mellanox/mlxsw/switchib.c b/drivers/net/ethernet/mellanox/mlxsw/switchib.c
>index ec0b27e..1552594 100644
>--- a/drivers/net/ethernet/mellanox/mlxsw/switchib.c
>+++ b/drivers/net/ethernet/mellanox/mlxsw/switchib.c
>@@ -43,7 +43,6 @@
> #include <linux/skbuff.h>
> #include <linux/if_vlan.h>
> #include <net/switchdev.h>
>-#include <generated/utsrelease.h>
I believe it is not needed so spectrum.c and switchx2.x. Would you
please extend your patch to remove from there as well? Thanks.
^ permalink raw reply
* Re: [PATCH net-next v2] ipv4: fib: Replay events when registering FIB notifier
From: Ido Schimmel @ 2016-11-01 15:44 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev, davem, jiri, mlxsw, roopa, dsa, nikolay, andy,
vivien.didelot, andrew, f.fainelli, alexander.h.duyck, kuznet,
jmorris, yoshfuji, kaber, Ido Schimmel
In-Reply-To: <1478009999.7065.334.camel@edumazet-glaptop3.roam.corp.google.com>
On Tue, Nov 01, 2016 at 07:19:59AM -0700, Eric Dumazet wrote:
> On Tue, 2016-11-01 at 00:57 +0200, Ido Schimmel wrote:
> > On Mon, Oct 31, 2016 at 02:24:06PM -0700, Eric Dumazet wrote:
>
> > > How well will this work for large FIB tables ?
> > >
> > > Holding rtnl while sending thousands of skb will prevent consumers to
> > > make progress ?
> >
> > Can you please clarify what do you mean by "while sending thousands of
> > skb"? This patch doesn't generate notifications to user space, but
> > instead invokes notification routines inside the kernel. I probably
> > misunderstood you.
> >
> > Are you suggesting this be done using RCU instead? Well, there are a
> > couple of reasons why I took RTNL here:
> >
>
> No, I do not believe RCU is wanted here, in control path where we might
> sleep anyway.
>
> > 1) The FIB notification chain is blocking, so listeners are expected to
> > be able to sleep. This isn't possible if we use RCU. Note that this
> > chain is mainly useful for drivers that reflect the FIB table into a
> > capable device and hardware operations usually involve sleeping.
> >
> > 2) The insertion of a single route is done with RTNL held. I didn't want
> > to differentiate between both cases. This property is really useful for
> > listeners, as they don't need to worry about locking in writer-side.
> > Access to data structs is serialized by RTNL.
>
> My concern was that for large iterations, you might hold RTNL and/or
> current cpu for hundred of ms or even seconds...
I understand your concern, but I think it's helpful to look at the users
of this API. It was only recently introduced [1] because nobody needed
it beside switch drivers that reflect the FIB table and I believe it'll
stay that way. Currently, only mlxsw and rocker use it.
Now, in these use cases when register_fib_notifier() is called the
switch ports are still not present in the system, so we really only have
a few routes used for management. Similarly, when
unregister_fib_notifier() is called, the switch ports are already gone
and most FIBs were flushed due to NETDEV_UNREGISTER, so again we only
have a handful of FIBs to iterate over.
Does that sound reasonable to you?
1. https://www.spinics.net/lists/netdev/msg397444.html
^ permalink raw reply
* Re: [PATCH net-next 5/5] ipv6: Compute multipath hash for forwarded ICMP errors from offending packet
From: Jakub Sitnicki @ 2016-11-01 15:43 UTC (permalink / raw)
To: Tom Herbert
Cc: Linux Kernel Network Developers, LKML, David S. Miller,
Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
Patrick McHardy
In-Reply-To: <CALx6S36r0gzZ7ndmivsygZD9vX+EEv2sVEP=rmhQVzGB46Rkdg@mail.gmail.com>
On Mon, Oct 31, 2016 at 07:25 PM GMT, Tom Herbert wrote:
> On Sun, Oct 30, 2016 at 6:03 AM, Jakub Sitnicki <jkbs@redhat.com> wrote:
>> On Fri, Oct 28, 2016 at 02:25 PM GMT, Tom Herbert wrote:
>>>
>>> If this patch is being done to be compatible with IPv4 I guess that's
>>> okay, but it would be false advertisement to say this makes ICMP
>>> follow the same path as the flow being targeted in an error.
>>> Fortunately, I doubt anyone can have a dependency on this for ICMP.
>>
>> I wouldn't want to propose anything that would be useless. If you think
>> that this is the case here, I would very much like to understand what
>> and why cannot work in practice.
>>
> The normal hash for TCP or UDP using ECMP is over <protocol, srcIP,
> dstIP, srcPort, dstPort>. For an ICMP packet ECMP would most likely be
> done over <protocol, srcIP, dstIP>. There really is no way to ensure
> that an ICMP packet will follow the same path as TCP or any other
> protocol. Fortunately, this is really isn't so terrible. The Internet
> has worked this way ever since routers started using ports as input to
> ECMP and that hasn't caused any major meltdown.
Ahh, I see the problem now. Thank you for clearing it up for me.
You are right, for locally generated TCP/UDP traffic we are computing an
L4 hash (over the 5-tuple that you mentioned) that drives the multipath
routing. While when sending locally generated ICMP errors we are only
computing an L3 hash (over the mentioned 3-tuple).
I believe that is a problem affects both IPv6 and IPv4, and manifests
itself only when the offending packet that triggers the error is
destined for the ECMP router.
When an ECMP router is: (i) sending an ICMP error in reaction to a
packet that was to be forwarded, or (ii) forwarding an ICMP error,
everything works as expected. That is because when forwarding traffic we
limit ourselves to computing an L3 hash so that the IP fragments are
routed together. Right?
So, my understanding is that, with these changes, things are not perfect
but we are not worse than IPv4 right now.
Would you be okay with this series if I update the patch 4/5 description
to highlight the existing problem that you point out? A fix for this
IPv4/6 common issue could follow afterwards.
Thanks,
Jakub
^ permalink raw reply
* Re: [PATCH v7 0/6] Add eBPF hooks for cgroups
From: David Miller @ 2016-11-01 15:38 UTC (permalink / raw)
To: lorenzo-hpIqsD4AKlfQT0dZR+AlfA
Cc: daniel-FeC+5ew28dpmcu3hnIyYJQ,
alexei.starovoitov-Re5JQEeQqe8AvxtiuMwx3w,
daniel-cYrQPVfZoowdnm+yROfE0A, pablo-Cap9r6Oaw4JrovVCs/uTlw,
htejun-b10kYP2dOMg, ast-b10kYP2dOMg, kafai-b10kYP2dOMg,
fw-HFFVJYpyMKqzQB+pC5nmwQ, harald-H+wXaHxf7aLQT0dZR+AlfA,
netdev-u79uwXL29TY76Z2rM5mHXA, sargun-GaZTRHToo+CzQB+pC5nmwQ,
cgroups-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CAKD1Yr02SCHvd-xZJL14d_Ta8Dk4evHZ60zytpU0h4r80FucwA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
From: Lorenzo Colitti <lorenzo-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
Date: Wed, 2 Nov 2016 00:25:15 +0900
> That way, if you want to modify the packet or do something
> sophisticated in netfilter, you can still use the eBPF hook on the
> results of that operation, and if you don't want to run netfilter, you
> can write netfilter rules to skip the packet (and maybe still fix it
> up later, perhaps in another netfilter chain).
The downside is that we classify the packet twice. This transactional
cost adds up rather quickly.
^ permalink raw reply
* Re: [PATCH net-next v2] ipv4: fib: Replay events when registering FIB notifier
From: David Miller @ 2016-11-01 15:36 UTC (permalink / raw)
To: roopa
Cc: eric.dumazet, idosch, netdev, jiri, mlxsw, dsa, nikolay, andy,
vivien.didelot, andrew, f.fainelli, alexander.h.duyck, kuznet,
jmorris, yoshfuji, kaber, idosch
In-Reply-To: <5818B146.20209@cumulusnetworks.com>
From: Roopa Prabhu <roopa@cumulusnetworks.com>
Date: Tue, 01 Nov 2016 08:14:14 -0700
> On 11/1/16, 7:19 AM, Eric Dumazet wrote:
>> On Tue, 2016-11-01 at 00:57 +0200, Ido Schimmel wrote:
>>> On Mon, Oct 31, 2016 at 02:24:06PM -0700, Eric Dumazet wrote:
>>>> How well will this work for large FIB tables ?
>>>>
>>>> Holding rtnl while sending thousands of skb will prevent consumers to
>>>> make progress ?
>>> Can you please clarify what do you mean by "while sending thousands of
>>> skb"? This patch doesn't generate notifications to user space, but
>>> instead invokes notification routines inside the kernel. I probably
>>> misunderstood you.
>>>
>>> Are you suggesting this be done using RCU instead? Well, there are a
>>> couple of reasons why I took RTNL here:
>>>
>> No, I do not believe RCU is wanted here, in control path where we might
>> sleep anyway.
>>
>>> 1) The FIB notification chain is blocking, so listeners are expected to
>>> be able to sleep. This isn't possible if we use RCU. Note that this
>>> chain is mainly useful for drivers that reflect the FIB table into a
>>> capable device and hardware operations usually involve sleeping.
>>>
>>> 2) The insertion of a single route is done with RTNL held. I didn't want
>>> to differentiate between both cases. This property is really useful for
>>> listeners, as they don't need to worry about locking in writer-side.
>>> Access to data structs is serialized by RTNL.
>> My concern was that for large iterations, you might hold RTNL and/or
>> current cpu for hundred of ms or even seconds...
>>
> I have the same concern as Eric here.
>
> I understand why you need it, but can the driver request for an initial dump and that
> dump be made more efficient somehow ie not hold rtnl for the whole dump ?.
> instead of making the fib notifier registration code doing it.
>
> these routing table sizes can be huge and an analogy for this in user-space:
> We do request a netlink dump of routing tables at initialization (on driver starts or resets)...
> but, existing netlink routing table dumps for that scale don't hold rtnl for the whole dump.
> The dump is split into multiple responses to the user and hence it does not starve other rtnl users.
>
> In-fact I don't think netlink routing table dumps from user-space hold rtnl_lock for the whole dump.
> IIRC this was done to allow route add/dels to be allowed in parallel for performance reasons.
> (I will need to double check to confirm this).
I've always had some reservations about using notifiers for getting
the FIB entries down to the offloaded device.
And this problem is just another symptom that it is the wrong
mechanism for propagating this information.
As suggested by Roopa here, perhaps we're looking at the problem from
the wrong direction. We tend to design NDO ops and notifiers, to
"push" things to the driver, but maybe something more like a push+pull
model is better.
^ permalink raw reply
* Re: [PATCH net-next 5/5] ipv6: Compute multipath hash for forwarded ICMP errors from offending packet
From: David Miller @ 2016-11-01 15:35 UTC (permalink / raw)
To: jkbs; +Cc: tom, netdev, linux-kernel, kuznet, jmorris, yoshfuji, kaber
In-Reply-To: <87bmxznsgg.fsf@redhat.com>
From: Jakub Sitnicki <jkbs@redhat.com>
Date: Tue, 01 Nov 2016 16:13:51 +0100
> On Mon, Oct 31, 2016 at 07:15 PM GMT, David Miller wrote:
>> From: Jakub Sitnicki <jkbs@redhat.com>
>> Date: Sun, 30 Oct 2016 14:03:11 +0100
>>
>>> 2) ensure the flow labels used in both directions are the same (either
>>> reflected by one side, or fixed, e.g. not used and set to 0), so that
>>> the 4-tuple we hash over when forwarding, <src addr, dst addr, flow
>>> label, next hdr>, is the same both ways, modulo the order of
>>> addresses.
>>
>> Even Linux, by default, does not do reflection.
>>
>> See the flowlabel_consistency sysctl, which we set by default to '1'.
>
> Yes, unfortunately, if Linux-based hosts are used as sending/receiving
> IPv6, ICMP error forwarding will not work out of the box. Users will be
> burdened with adjusting the runtime network stack config, as you point
> out, or otherwise instructing the apps to set the flow label,
> e.g. traceroute6 -I <flow label> ...
I'm pretty sure that sysctl default was choosen intentionally, and we
actively are _encouraging_ the world to not depend upon reflection in
any way, shape, or form.
And it's kind of pointless to suggest a facility that can't work with
Linux endpoints out of the box.
This was the point I'm trying to make.
If the intentions of that sysctl default does pan out, the idea is for
the world to move towards arbitrary flow label settings, even perhaps
changing over time. The intention is to make this more, not less,
common. And the idea is to give maximum flexibility for endpoints to
set these flow labels, in order to increase entropy wherever possible.
I have a really hard time accepting a "fix" that depends upon behavior
that the Linux ipv6 stack doesn't even have.
^ permalink raw reply
* [PATCH 3/3] net: 3c509: use new api ethtool_{get|set}_link_ksettings
From: Philippe Reynes @ 2016-11-01 15:32 UTC (permalink / raw)
To: klassert, davem, mugunthanvnm, a, fw, jarod, fabf
Cc: netdev, linux-kernel, Philippe Reynes
In-Reply-To: <1478014347-14994-1-git-send-email-tremyfr@gmail.com>
The ethtool api {get|set}_settings is deprecated.
We move this driver to new api {get|set}_link_ksettings.
Signed-off-by: Philippe Reynes <tremyfr@gmail.com>
---
drivers/net/ethernet/3com/3c509.c | 55 +++++++++++++++++++-----------------
1 files changed, 29 insertions(+), 26 deletions(-)
diff --git a/drivers/net/ethernet/3com/3c509.c b/drivers/net/ethernet/3com/3c509.c
index 9f9a5f4..a753378 100644
--- a/drivers/net/ethernet/3com/3c509.c
+++ b/drivers/net/ethernet/3com/3c509.c
@@ -1040,67 +1040,68 @@ static void update_stats(struct net_device *dev)
}
static int
-el3_netdev_get_ecmd(struct net_device *dev, struct ethtool_cmd *ecmd)
+el3_netdev_get_ecmd(struct net_device *dev, struct ethtool_link_ksettings *cmd)
{
u16 tmp;
int ioaddr = dev->base_addr;
+ u32 supported;
EL3WINDOW(0);
/* obtain current transceiver via WN4_MEDIA? */
tmp = inw(ioaddr + WN0_ADDR_CONF);
- ecmd->transceiver = XCVR_INTERNAL;
switch (tmp >> 14) {
case 0:
- ecmd->port = PORT_TP;
+ cmd->base.port = PORT_TP;
break;
case 1:
- ecmd->port = PORT_AUI;
- ecmd->transceiver = XCVR_EXTERNAL;
+ cmd->base.port = PORT_AUI;
break;
case 3:
- ecmd->port = PORT_BNC;
+ cmd->base.port = PORT_BNC;
default:
break;
}
- ecmd->duplex = DUPLEX_HALF;
- ecmd->supported = 0;
+ cmd->base.duplex = DUPLEX_HALF;
+ supported = 0;
tmp = inw(ioaddr + WN0_CONF_CTRL);
if (tmp & (1<<13))
- ecmd->supported |= SUPPORTED_AUI;
+ supported |= SUPPORTED_AUI;
if (tmp & (1<<12))
- ecmd->supported |= SUPPORTED_BNC;
+ supported |= SUPPORTED_BNC;
if (tmp & (1<<9)) {
- ecmd->supported |= SUPPORTED_TP | SUPPORTED_10baseT_Half |
+ supported |= SUPPORTED_TP | SUPPORTED_10baseT_Half |
SUPPORTED_10baseT_Full; /* hmm... */
EL3WINDOW(4);
tmp = inw(ioaddr + WN4_NETDIAG);
if (tmp & FD_ENABLE)
- ecmd->duplex = DUPLEX_FULL;
+ cmd->base.duplex = DUPLEX_FULL;
}
- ethtool_cmd_speed_set(ecmd, SPEED_10);
+ ethtool_convert_legacy_u32_to_link_mode(cmd->link_modes.supported,
+ supported);
+ cmd->base.speed = SPEED_10;
EL3WINDOW(1);
return 0;
}
static int
-el3_netdev_set_ecmd(struct net_device *dev, struct ethtool_cmd *ecmd)
+el3_netdev_set_ecmd(struct net_device *dev,
+ const struct ethtool_link_ksettings *cmd)
{
u16 tmp;
int ioaddr = dev->base_addr;
- if (ecmd->speed != SPEED_10)
+ if (cmd->base.speed != SPEED_10)
return -EINVAL;
- if ((ecmd->duplex != DUPLEX_HALF) && (ecmd->duplex != DUPLEX_FULL))
- return -EINVAL;
- if ((ecmd->transceiver != XCVR_INTERNAL) && (ecmd->transceiver != XCVR_EXTERNAL))
+ if ((cmd->base.duplex != DUPLEX_HALF) &&
+ (cmd->base.duplex != DUPLEX_FULL))
return -EINVAL;
/* change XCVR type */
EL3WINDOW(0);
tmp = inw(ioaddr + WN0_ADDR_CONF);
- switch (ecmd->port) {
+ switch (cmd->base.port) {
case PORT_TP:
tmp &= ~(3<<14);
dev->if_port = 0;
@@ -1130,7 +1131,7 @@ static void update_stats(struct net_device *dev)
EL3WINDOW(4);
tmp = inw(ioaddr + WN4_NETDIAG);
- if (ecmd->duplex == DUPLEX_FULL)
+ if (cmd->base.duplex == DUPLEX_FULL)
tmp |= FD_ENABLE;
else
tmp &= ~FD_ENABLE;
@@ -1146,24 +1147,26 @@ static void el3_get_drvinfo(struct net_device *dev, struct ethtool_drvinfo *info
strlcpy(info->version, DRV_VERSION, sizeof(info->version));
}
-static int el3_get_settings(struct net_device *dev, struct ethtool_cmd *ecmd)
+static int el3_get_link_ksettings(struct net_device *dev,
+ struct ethtool_link_ksettings *cmd)
{
struct el3_private *lp = netdev_priv(dev);
int ret;
spin_lock_irq(&lp->lock);
- ret = el3_netdev_get_ecmd(dev, ecmd);
+ ret = el3_netdev_get_ecmd(dev, cmd);
spin_unlock_irq(&lp->lock);
return ret;
}
-static int el3_set_settings(struct net_device *dev, struct ethtool_cmd *ecmd)
+static int el3_set_link_ksettings(struct net_device *dev,
+ const struct ethtool_link_ksettings *cmd)
{
struct el3_private *lp = netdev_priv(dev);
int ret;
spin_lock_irq(&lp->lock);
- ret = el3_netdev_set_ecmd(dev, ecmd);
+ ret = el3_netdev_set_ecmd(dev, cmd);
spin_unlock_irq(&lp->lock);
return ret;
}
@@ -1191,11 +1194,11 @@ static void el3_set_msglevel(struct net_device *dev, u32 v)
static const struct ethtool_ops ethtool_ops = {
.get_drvinfo = el3_get_drvinfo,
- .get_settings = el3_get_settings,
- .set_settings = el3_set_settings,
.get_link = el3_get_link,
.get_msglevel = el3_get_msglevel,
.set_msglevel = el3_set_msglevel,
+ .get_link_ksettings = el3_get_link_ksettings,
+ .set_link_ksettings = el3_set_link_ksettings,
};
static void
--
1.7.4.4
^ permalink raw reply related
* [PATCH net-next] tcp: enhance tcp collapsing
From: Eric Dumazet @ 2016-11-01 15:32 UTC (permalink / raw)
To: Ilya Lesokhin, David Miller
Cc: netdev@vger.kernel.org, Ilpo Järvinen, Neal Cardwell,
Yuchung Cheng
In-Reply-To: <1478007189.7065.331.camel@edumazet-glaptop3.roam.corp.google.com>
From: Eric Dumazet <edumazet@google.com>
As Ilya Lesokhin suggested, we can collapse two skbs at retransmit
time even if the skb at the right has fragments.
We simply have to use more generic skb_copy_bits() instead of
skb_copy_from_linear_data() in tcp_collapse_retrans()
Tested:
Used following packetdrill test
// Establish a connection.
0.000 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
+0 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
+0 bind(3, ..., ...) = 0
+0 listen(3, 1) = 0
+0 < S 0:0(0) win 32792 <mss 1460,sackOK,nop,nop,nop,wscale 8>
+0 > S. 0:0(0) ack 1 <mss 1460,nop,nop,sackOK,nop,wscale 8>
+.100 < . 1:1(0) ack 1 win 257
+0 accept(3, ..., ...) = 4
+0 setsockopt(4, SOL_TCP, TCP_NODELAY, [1], 4) = 0
+0 write(4, ..., 200) = 200
+0 > P. 1:201(200) ack 1
+.001 write(4, ..., 200) = 200
+0 > P. 201:401(200) ack 1
+.001 write(4, ..., 200) = 200
+0 > P. 401:601(200) ack 1
+.001 write(4, ..., 200) = 200
+0 > P. 601:801(200) ack 1
+.001 write(4, ..., 200) = 200
+0 > P. 801:1001(200) ack 1
+.001 write(4, ..., 100) = 100
+0 > P. 1001:1101(100) ack 1
+.001 write(4, ..., 100) = 100
+0 > P. 1101:1201(100) ack 1
+.001 write(4, ..., 100) = 100
+0 > P. 1201:1301(100) ack 1
+.001 write(4, ..., 100) = 100
+0 > P. 1301:1401(100) ack 1
+.100 < . 1:1(0) ack 1 win 257 <nop,nop,sack 1001:1401>
// Check that TCP collapse works :
+0 > P. 1:1001(1000) ack 1
Reported-by: Ilya Lesokhin <ilyal@mellanox.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Neal Cardwell <ncardwell@google.com>
---
net/ipv4/tcp_output.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 896e9dfbdb5cd9ca0fa003f6be2c5cd332dde7cf..ad668d97fbb1660a9ad44ddb459ea45e15a22de1 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -2529,8 +2529,7 @@ static void tcp_collapse_retrans(struct sock *sk, struct sk_buff *skb)
tcp_unlink_write_queue(next_skb, sk);
- skb_copy_from_linear_data(next_skb, skb_put(skb, next_skb_size),
- next_skb_size);
+ skb_copy_bits(next_skb, 0, skb_put(skb, next_skb_size), next_skb_size);
if (next_skb->ip_summed == CHECKSUM_PARTIAL)
skb->ip_summed = CHECKSUM_PARTIAL;
@@ -2567,14 +2566,11 @@ static bool tcp_can_collapse(const struct sock *sk, const struct sk_buff *skb)
{
if (tcp_skb_pcount(skb) > 1)
return false;
- /* TODO: SACK collapsing could be used to remove this condition */
- if (skb_shinfo(skb)->nr_frags != 0)
- return false;
if (skb_cloned(skb))
return false;
if (skb == tcp_send_head(sk))
return false;
- /* Some heurestics for collapsing over SACK'd could be invented */
+ /* Some heuristics for collapsing over SACK'd could be invented */
if (TCP_SKB_CB(skb)->sacked & TCPCB_SACKED_ACKED)
return false;
^ permalink raw reply related
* [PATCH 2/3] net: 3c59x: use new api ethtool_{get|set}_link_ksettings
From: Philippe Reynes @ 2016-11-01 15:32 UTC (permalink / raw)
To: klassert, davem, mugunthanvnm, a, fw, jarod, fabf
Cc: netdev, linux-kernel, Philippe Reynes
In-Reply-To: <1478014347-14994-1-git-send-email-tremyfr@gmail.com>
The ethtool api {get|set}_settings is deprecated.
We move this driver to new api {get|set}_link_ksettings.
Signed-off-by: Philippe Reynes <tremyfr@gmail.com>
---
drivers/net/ethernet/3com/3c59x.c | 14 ++++++++------
1 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/3com/3c59x.c b/drivers/net/ethernet/3com/3c59x.c
index 3ecf613..b3560a3 100644
--- a/drivers/net/ethernet/3com/3c59x.c
+++ b/drivers/net/ethernet/3com/3c59x.c
@@ -2907,18 +2907,20 @@ static int vortex_nway_reset(struct net_device *dev)
return mii_nway_restart(&vp->mii);
}
-static int vortex_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
+static int vortex_get_link_ksettings(struct net_device *dev,
+ struct ethtool_link_ksettings *cmd)
{
struct vortex_private *vp = netdev_priv(dev);
- return mii_ethtool_gset(&vp->mii, cmd);
+ return mii_ethtool_get_link_ksettings(&vp->mii, cmd);
}
-static int vortex_set_settings(struct net_device *dev, struct ethtool_cmd *cmd)
+static int vortex_set_link_ksettings(struct net_device *dev,
+ const struct ethtool_link_ksettings *cmd)
{
struct vortex_private *vp = netdev_priv(dev);
- return mii_ethtool_sset(&vp->mii, cmd);
+ return mii_ethtool_set_link_ksettings(&vp->mii, cmd);
}
static u32 vortex_get_msglevel(struct net_device *dev)
@@ -3031,13 +3033,13 @@ static int vortex_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
.set_msglevel = vortex_set_msglevel,
.get_ethtool_stats = vortex_get_ethtool_stats,
.get_sset_count = vortex_get_sset_count,
- .get_settings = vortex_get_settings,
- .set_settings = vortex_set_settings,
.get_link = ethtool_op_get_link,
.nway_reset = vortex_nway_reset,
.get_wol = vortex_get_wol,
.set_wol = vortex_set_wol,
.get_ts_info = ethtool_op_get_ts_info,
+ .get_link_ksettings = vortex_get_link_ksettings,
+ .set_link_ksettings = vortex_set_link_ksettings,
};
#ifdef CONFIG_PCI
--
1.7.4.4
^ permalink raw reply related
* [PATCH 1/3] net: mii: add generic function to support ksetting support
From: Philippe Reynes @ 2016-11-01 15:32 UTC (permalink / raw)
To: klassert, davem, mugunthanvnm, a, fw, jarod, fabf
Cc: netdev, linux-kernel, Philippe Reynes
The old ethtool api (get_setting and set_setting) has generic mii
functions mii_ethtool_sset and mii_ethtool_gset.
To support the new ethtool api ({get|set}_link_ksettings), we add
two generics mii function mii_ethtool_{get|set}_link_ksettings_get.
Signed-off-by: Philippe Reynes <tremyfr@gmail.com>
---
drivers/net/mii.c | 195 +++++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/mii.h | 4 +
2 files changed, 199 insertions(+), 0 deletions(-)
diff --git a/drivers/net/mii.c b/drivers/net/mii.c
index 993570b..0443546 100644
--- a/drivers/net/mii.c
+++ b/drivers/net/mii.c
@@ -135,6 +135,101 @@ int mii_ethtool_gset(struct mii_if_info *mii, struct ethtool_cmd *ecmd)
}
/**
+ * mii_ethtool_get_link_ksettings - get settings that are specified in @cmd
+ * @mii: MII interface
+ * @cmd: requested ethtool_link_ksettings
+ *
+ * The @cmd parameter is expected to have been cleared before calling
+ * mii_ethtool_get_link_ksettings().
+ *
+ * Returns 0 for success, negative on error.
+ */
+int mii_ethtool_get_link_ksettings(struct mii_if_info *mii,
+ struct ethtool_link_ksettings *cmd)
+{
+ struct net_device *dev = mii->dev;
+ u16 bmcr, bmsr, ctrl1000 = 0, stat1000 = 0;
+ u32 nego, supported, advertising, lp_advertising;
+
+ supported = (SUPPORTED_10baseT_Half | SUPPORTED_10baseT_Full |
+ SUPPORTED_100baseT_Half | SUPPORTED_100baseT_Full |
+ SUPPORTED_Autoneg | SUPPORTED_TP | SUPPORTED_MII);
+ if (mii->supports_gmii)
+ supported |= SUPPORTED_1000baseT_Half |
+ SUPPORTED_1000baseT_Full;
+
+ /* only supports twisted-pair */
+ cmd->base.port = PORT_MII;
+
+ /* this isn't fully supported at higher layers */
+ cmd->base.phy_address = mii->phy_id;
+ cmd->base.mdio_support = ETH_MDIO_SUPPORTS_C22;
+
+ advertising = ADVERTISED_TP | ADVERTISED_MII;
+
+ bmcr = mii->mdio_read(dev, mii->phy_id, MII_BMCR);
+ bmsr = mii->mdio_read(dev, mii->phy_id, MII_BMSR);
+ if (mii->supports_gmii) {
+ ctrl1000 = mii->mdio_read(dev, mii->phy_id, MII_CTRL1000);
+ stat1000 = mii->mdio_read(dev, mii->phy_id, MII_STAT1000);
+ }
+ if (bmcr & BMCR_ANENABLE) {
+ advertising |= ADVERTISED_Autoneg;
+ cmd->base.autoneg = AUTONEG_ENABLE;
+
+ advertising |= mii_get_an(mii, MII_ADVERTISE);
+ if (mii->supports_gmii)
+ advertising |= mii_ctrl1000_to_ethtool_adv_t(ctrl1000);
+
+ if (bmsr & BMSR_ANEGCOMPLETE) {
+ lp_advertising = mii_get_an(mii, MII_LPA);
+ lp_advertising |=
+ mii_stat1000_to_ethtool_lpa_t(stat1000);
+ } else {
+ lp_advertising = 0;
+ }
+
+ nego = advertising & lp_advertising;
+
+ if (nego & (ADVERTISED_1000baseT_Full |
+ ADVERTISED_1000baseT_Half)) {
+ cmd->base.speed = SPEED_1000;
+ cmd->base.duplex = !!(nego & ADVERTISED_1000baseT_Full);
+ } else if (nego & (ADVERTISED_100baseT_Full |
+ ADVERTISED_100baseT_Half)) {
+ cmd->base.speed = SPEED_100;
+ cmd->base.duplex = !!(nego & ADVERTISED_100baseT_Full);
+ } else {
+ cmd->base.speed = SPEED_10;
+ cmd->base.duplex = !!(nego & ADVERTISED_10baseT_Full);
+ }
+ } else {
+ cmd->base.autoneg = AUTONEG_DISABLE;
+
+ cmd->base.speed = ((bmcr & BMCR_SPEED1000 &&
+ (bmcr & BMCR_SPEED100) == 0) ?
+ SPEED_1000 :
+ ((bmcr & BMCR_SPEED100) ?
+ SPEED_100 : SPEED_10));
+ cmd->base.duplex = (bmcr & BMCR_FULLDPLX) ?
+ DUPLEX_FULL : DUPLEX_HALF;
+ }
+
+ mii->full_duplex = cmd->base.duplex;
+
+ ethtool_convert_legacy_u32_to_link_mode(cmd->link_modes.supported,
+ supported);
+ ethtool_convert_legacy_u32_to_link_mode(cmd->link_modes.advertising,
+ advertising);
+ ethtool_convert_legacy_u32_to_link_mode(cmd->link_modes.lp_advertising,
+ lp_advertising);
+
+ /* ignore maxtxpkt, maxrxpkt for now */
+
+ return 0;
+}
+
+/**
* mii_ethtool_sset - set settings that are specified in @ecmd
* @mii: MII interface
* @ecmd: requested ethtool_cmd
@@ -227,6 +322,104 @@ int mii_ethtool_sset(struct mii_if_info *mii, struct ethtool_cmd *ecmd)
}
/**
+ * mii_ethtool_set_link_ksettings - set settings that are specified in @cmd
+ * @mii: MII interfaces
+ * @cmd: requested ethtool_link_ksettings
+ *
+ * Returns 0 for success, negative on error.
+ */
+int mii_ethtool_set_link_ksettings(struct mii_if_info *mii,
+ const struct ethtool_link_ksettings *cmd)
+{
+ struct net_device *dev = mii->dev;
+ u32 speed = cmd->base.speed;
+
+ if (speed != SPEED_10 &&
+ speed != SPEED_100 &&
+ speed != SPEED_1000)
+ return -EINVAL;
+ if (cmd->base.duplex != DUPLEX_HALF && cmd->base.duplex != DUPLEX_FULL)
+ return -EINVAL;
+ if (cmd->base.port != PORT_MII)
+ return -EINVAL;
+ if (cmd->base.phy_address != mii->phy_id)
+ return -EINVAL;
+ if (cmd->base.autoneg != AUTONEG_DISABLE &&
+ cmd->base.autoneg != AUTONEG_ENABLE)
+ return -EINVAL;
+ if ((speed == SPEED_1000) && (!mii->supports_gmii))
+ return -EINVAL;
+
+ /* ignore supported, maxtxpkt, maxrxpkt */
+
+ if (cmd->base.autoneg == AUTONEG_ENABLE) {
+ u32 bmcr, advert, tmp;
+ u32 advert2 = 0, tmp2 = 0;
+ u32 advertising;
+
+ ethtool_convert_link_mode_to_legacy_u32(
+ &advertising, cmd->link_modes.advertising);
+
+ if ((advertising & (ADVERTISED_10baseT_Half |
+ ADVERTISED_10baseT_Full |
+ ADVERTISED_100baseT_Half |
+ ADVERTISED_100baseT_Full |
+ ADVERTISED_1000baseT_Half |
+ ADVERTISED_1000baseT_Full)) == 0)
+ return -EINVAL;
+
+ /* advertise only what has been requested */
+ advert = mii->mdio_read(dev, mii->phy_id, MII_ADVERTISE);
+ tmp = advert & ~(ADVERTISE_ALL | ADVERTISE_100BASE4);
+ if (mii->supports_gmii) {
+ advert2 = mii->mdio_read(dev, mii->phy_id,
+ MII_CTRL1000);
+ tmp2 = advert2 &
+ ~(ADVERTISE_1000HALF | ADVERTISE_1000FULL);
+ }
+ tmp |= ethtool_adv_to_mii_adv_t(advertising);
+
+ if (mii->supports_gmii)
+ tmp2 |= ethtool_adv_to_mii_ctrl1000_t(advertising);
+ if (advert != tmp) {
+ mii->mdio_write(dev, mii->phy_id, MII_ADVERTISE, tmp);
+ mii->advertising = tmp;
+ }
+ if ((mii->supports_gmii) && (advert2 != tmp2))
+ mii->mdio_write(dev, mii->phy_id, MII_CTRL1000, tmp2);
+
+ /* turn on autonegotiation, and force a renegotiate */
+ bmcr = mii->mdio_read(dev, mii->phy_id, MII_BMCR);
+ bmcr |= (BMCR_ANENABLE | BMCR_ANRESTART);
+ mii->mdio_write(dev, mii->phy_id, MII_BMCR, bmcr);
+
+ mii->force_media = 0;
+ } else {
+ u32 bmcr, tmp;
+
+ /* turn off auto negotiation, set speed and duplexity */
+ bmcr = mii->mdio_read(dev, mii->phy_id, MII_BMCR);
+ tmp = bmcr & ~(BMCR_ANENABLE | BMCR_SPEED100 |
+ BMCR_SPEED1000 | BMCR_FULLDPLX);
+ if (speed == SPEED_1000)
+ tmp |= BMCR_SPEED1000;
+ else if (speed == SPEED_100)
+ tmp |= BMCR_SPEED100;
+ if (cmd->base.duplex == DUPLEX_FULL) {
+ tmp |= BMCR_FULLDPLX;
+ mii->full_duplex = 1;
+ } else {
+ mii->full_duplex = 0;
+ }
+ if (bmcr != tmp)
+ mii->mdio_write(dev, mii->phy_id, MII_BMCR, tmp);
+
+ mii->force_media = 1;
+ }
+ return 0;
+}
+
+/**
* mii_check_gmii_support - check if the MII supports Gb interfaces
* @mii: the MII interface
*/
@@ -466,7 +659,9 @@ int generic_mii_ioctl(struct mii_if_info *mii_if,
EXPORT_SYMBOL(mii_link_ok);
EXPORT_SYMBOL(mii_nway_restart);
EXPORT_SYMBOL(mii_ethtool_gset);
+EXPORT_SYMBOL(mii_ethtool_get_link_ksettings);
EXPORT_SYMBOL(mii_ethtool_sset);
+EXPORT_SYMBOL(mii_ethtool_set_link_ksettings);
EXPORT_SYMBOL(mii_check_link);
EXPORT_SYMBOL(mii_check_media);
EXPORT_SYMBOL(mii_check_gmii_support);
diff --git a/include/linux/mii.h b/include/linux/mii.h
index 47492c9..1629a0c 100644
--- a/include/linux/mii.h
+++ b/include/linux/mii.h
@@ -31,7 +31,11 @@ struct mii_if_info {
extern int mii_link_ok (struct mii_if_info *mii);
extern int mii_nway_restart (struct mii_if_info *mii);
extern int mii_ethtool_gset(struct mii_if_info *mii, struct ethtool_cmd *ecmd);
+extern int mii_ethtool_get_link_ksettings(
+ struct mii_if_info *mii, struct ethtool_link_ksettings *cmd);
extern int mii_ethtool_sset(struct mii_if_info *mii, struct ethtool_cmd *ecmd);
+extern int mii_ethtool_set_link_ksettings(
+ struct mii_if_info *mii, const struct ethtool_link_ksettings *cmd);
extern int mii_check_gmii_support(struct mii_if_info *mii);
extern void mii_check_link (struct mii_if_info *mii);
extern unsigned int mii_check_media (struct mii_if_info *mii,
--
1.7.4.4
^ permalink raw reply related
* Re: Let's do P4
From: John Fastabend @ 2016-11-01 15:13 UTC (permalink / raw)
To: Jiri Pirko
Cc: Alexei Starovoitov, Thomas Graf, Jakub Kicinski, netdev, davem,
jhs, roopa, simon.horman, ast, daniel, prem, hannes, jbenc, tom,
mattyk, idosch, eladr, yotamg, nogahf, ogerlitz, linville, andy,
f.fainelli, dsa, vivien.didelot, andrew, ivecera,
Maciej Żenczykowski
In-Reply-To: <20161101084643.GA1707@nanopsycho.orion>
[...]
>>> P4 is ment to program programable hw, not fixed pipeline.
>>>
>>
>> I'm guessing there are no upstream drivers at the moment that support
>> this though right? The rocker universe bits though could leverage this.
>
> mlxsw. But this is naturaly not implemented yet, as there is no
> infrastructure.
Really? What is re-programmable?
Can the parse graph support arbitrary parse graph?
Can the table topology be reconfigured?
Can new tables be created?
What about "new" actions being defined at configuration time?
Or is this just the normal TCAM configuration of defining key widths and
fields.
>
>
>>
>>>
>>>>
>>>>>
>>>>>> since I cannot see how one can put the whole p4 language compiler
>>>>>> into the driver, so this last step of p4ast->hw, I presume, will be
>>>>>> done by firmware, which will be running full compiler in an embedded cpu
>>>>>
>>>>> In case of mlxsw, that compiler would be in driver.
>>>>>
>>>>>
>>>>>> on the switch. To me that's precisely the kernel bypass, since we won't
>>>>>> have a clue what HW capabilities actually are and won't be able to fine
>>>>>> grain control them.
>>>>>> Please correct me if I'm wrong.
>>>>>
>>>>> You are wrong. By your definition, everything has to be figured out in
>>>>> driver and FW does nothing. Otherwise it could do "something else" and
>>>>> that would be a bypass? Does not make any sense to me whatsoever.
>>>>>
>>>>>
>>>>>>
>>>>>>> Plus the thing I cannot imagine in the model you propose is table fillup.
>>>>>>> For ebpf, you use maps. For p4 you would have to have a separate HW-only
>>>>>>> API. This is very similar to the original John's Flow-API. And therefore
>>>>>>> a kernel bypass.
>>>>>>
>>>>>> I think John's flow api is a better way to expose mellanox switch capabilities.
>>>>>
>>>>> We are under impression that p4 suits us nicely. But it is not about
>>>>> us, it is about finding the common way to do this.
>>>>>
>>>>
>>>> I'll just poke at my FlowAPI question again. For fixed ASICS what is
>>>> the Flow-API missing. We have a few proof points that show it is both
>>>> sufficient and usable for the handful of use cases we care about.
>>>
>>> Yeah, it is most probably fine. Even for flex ASICs to some point. The
>>> question is how it stands comparing to other alternatives, like p4
>>>
>>
>> Just to be clear the Flow-API _was_ generated from the initial P4 spec.
>> The header files and tools used with it were autogenerated ("compiled"
>> in a loose sense) from the P4 program. The piece I never exposed
>> was the set_* operations to reconfigure running systems. I'm not sure
>> how valuable this is in practice though.
>>
>> Also there is a P4-16 spec that will be released shortly that is more
>> flexible and also more complex.
>
> Would it be able to easily extend the Flow-API to include the changes?
>
P4-16 will allow externs, "functions" to execute in the control flow and
possibly inside the parse graph. None of this was considered in the
Flow-API. So none of this is supported.
I still have the question are you trying to push the "programming" of
the device via 'tc' or just the runtime configuration of tables? If it
is just runtime Flow-API is sufficient IMO. If its programming the
device using the complete P4-16 spec than no its not sufficient. But
I don't believe vendors will expose the complete programmability of the
device in the driver, this is going to look more like a fw update than
a runtime change at least on the devices I'm aware of.
>
>>
>>>
>>>>
>>>>>
>>>>>> I also think it's not fair to call it 'bypass'. I see nothing in it
>>>>>> that justify such 'swear word' ;)
>>>>>
>>>>> John's Flow-API was a kernel bypass. Why? It was a API specifically
>>>>> designed to directly work with HW tables, without kernel being involved.
>>>>
>>>> I don't think that is a fair definition of HW bypass. The SKIP_SW flag
>>>> does exactly that for 'tc' based offloads and it was not rejected.
>>>
>>> No, no, no. You still have possibility to do the same thing in kernel,
>>> same functionality, with the same API. That is a big difference.
>>>
>>>
>>>>
>>>> The _real_ reason that seems to have fallen out of this and other
>>>> discussion is the Flow-API didn't provide an in-kernel translation into
>>>> an emulated patch. Note we always had a usermode translation to eBPF.
>>>> A secondary reason appears to be overhead of adding yet another netlink
>>>> family.
>>>
>>> Yeah. Maybe you remember, back then when Flow-API was being discussed,
>>> I suggested to wrap it under TC as cls_xflows and cls_xflowsaction of
>>> some sort and do in-kernel datapath implementation. I believe that after
>>> that, it would be acceptable.
>>>
>>
>> As I understand the thread here that is exactly the proposal here right?
>> With a discussion around if the structures/etc are sufficient or any
>> alternative representations exist.
>
> Might be the way, yes. But I fear that with other p4 extensions this
> might not be easy to align with. Therefore I though about something more
> generic, like the p4ast.
>
Same question as above are we _really_ talking about pushing the entire
programmability of the device via 'tc'. If so we need to have a vendor
say they will support and implement this?
>
>>
>>>
>>>>
>>>>>
>>>>>
>>>>>> The goal of flow api was to expose HW features to user space, so that
>>>>>> user space can program it. For something simple as mellanox switch
>>>>>> asic it fits perfectly well.
>>>>>
>>>>> Again, this is not mlx-asic-specific. And again, that is a kernel bypass.
>>>>>
>>>>>
>>>>>> Unless I misunderstand the bigger goal of this discussion and it's
>>>>>> about programming ezchip devices.
>>>>>
>>>>> No. For network processors, I believe that BPF is nicely offloadable, no
>>>>> need to do the excercise for that.
>>>>>
>>>>>
>>>>>>
>>>>>> If the goal is to model hw tcam in the linux kernel then just introduce
>>>>>> tcam bpf map type. It will be dog slow in user space, but it will
>>>>>> match exactly what is happnening in the HW and user space can make
>>>>>> sensible trade-offs.
>>>>>
>>>>> No, you got me completely wrong. This is not about the TCAM. This is
>>>>> about differences in the 2 words (p4/bpf).
>>>>> Again, for "p4-ish" devices, you have to translate BPF. And as you
>>>>> noted, it's an instruction set. Very hard if not impossible to parse in
>>>>> order to get back the original semantics.
>>>>>
>>>>
>>>> I think in this discussion "p4-ish" devices means devices with multiple
>>>> tables in a pipeline? Not devices that have programmable/configurable
>>>> pipelines right? And if we get to talking about reconfigurable devices
>>>> I believe this should be done out of band as it typically means
>>>> reloading some ucode, etc.
>>>
>>> I'm talking about both. But I think we should focus on reconfigurable
>>> ones, as we probably won't see that much fixed ones in the future.
>>>
>>
>> hmm maybe but the 10/40/100Gbps devices are going to be around for some
>> time. So we need to ensure these work well.
>
> Yes, but I would like to emphasize, if we are defining new api
> the primary focus should be on new devices.
>
>
What device though. Back to mlxsw question about actually supporting
this stuff.
^ 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