Re: [patch net-next v2 0/9] Introduce devlink interface and first drivers to use it
From: Jiri Pirko Date: Tue, 23 Feb 2016 16:51:25 +0100 > There a is need for some userspace API that would allow to expose things > that are not directly related to any device class like net_device of > ib_device, but rather chip-wide/switch-ASIC-wide stuff. > > Use cases: > 1) get/set of port type (Ethernet/InfiniBand) > 2) setting up port splitters - split port into multiple ones and squash again, >enables usage of splitter cable > 3) setting up shared buffers - shared among multiple ports within >one chip (work in progress) > 4) configuration of switch wide properties - resources division etc - This > will >allow to pass configuration that is unacceptable to be passed as >a module option. > > First patch of this set introduces a new generic Netlink based interface, > called "devlink". It is similar to nl80211 model and it is heavily > influenced by it, including the API definition. The devlink introduction patch > implements use cases 1) and 2). Other 2 are in development atm and will > be addressed by follow-ups. > > It is very convenient for drivers to use devlink, as you can see in other > patches in this set. > > Counterpart for devlink is userspace tool for now called "dl". Command line > interface and outputs are derived from "ip" tool so it should be easy > for users to get used to it. I am very close to applying this series as-is. The clincher for me is that there is precendence in the nl80211 stuff, so obviously whatever userland infrastructure sits on top of that has found a way to deal whatever perceived shortcomings devlink has. If all that people can come up with is "device names! omg UDEV!" and "multiple tools are hard to use!" That's awesome, because it means nobody has any real substantial objection to this facility. :-)
Re: [net-next PATCH 0/2] GENEVE/VXLAN: Enable outer Tx checksum by default
From: Tom Herbert Date: Tue, 23 Feb 2016 08:47:30 -0800 > Right, GRO should probably not coalesce packets with non-zero IP > identifiers due to the loss of information. If they are monotonically increasing, which is the only case worth caring about, it absolutely should. Because that can be done without any loss of information.
Re: [PATCH net-next 0/5] vxlan: consolidate rx handling
From: Jiri Benc Date: Tue, 23 Feb 2016 18:02:54 +0100 > Currently, vxlan_rcv is just called at the end of vxlan_udp_encap_recv, > continuing the rx processing where vxlan_udp_encap_recv left it. There's no > clear border between those two functions. This patchset moves > vxlan_udp_encap_recv and vxlan_rcv into a single function. > > This also allows to do some simplification in error path. > > The VXLAN-GPE implementation that will follow up this set can be seen at: > https://github.com/jbenc/linux-vxlan/commits/master Series applied, thank you.
Re: [PATCH net 1/5] ppp: lock ppp structure before modifying mru in ppp_ioctl()
From: Guillaume Nault Date: Thu, 25 Feb 2016 20:16:55 +0100 > Understood. Just to be sure, does patch #2 falls under lack of > demonstration? Or should I repost it separately? Please repost it if you feel this way, with the race and corruption possibility explained in detail in the commit log message. Thanks.
Re: Sending short raw packets using sendmsg() broke
From: Heikki Hannikainen Date: Thu, 25 Feb 2016 21:36:07 +0200 (EET) > Commit 9c7077622dd9174 added a check, ll_header_truncated(), which > requires that a packet transmitted using sendmsg() with PF_PACKET, > SOCK_RAW must be longer than dev->hard_header_len. Fixed by: commit 880621c2605b82eb5af91a2c94223df6f5a3fb64 Author: Martin Blumenstingl Date: Sun Nov 22 17:46:09 2015 +0100 packet: Allow packets with only a header (but no payload)
[PATCH] socket.7: Document some BPF-related socket options
From: Craig Gallek Document the behavior and the first kernel version for each of the following socket options: SO_ATTACH_FILTER SO_ATTACH_BPF SO_ATTACH_REUSEPORT_CBPF SO_ATTACH_REUSEPORT_EBPF SO_DETACH_FILTER SO_DETACH_BPF Signed-off-by: Craig Gallek --- man7/socket.7 | 104 -- 1 file changed, 86 insertions(+), 18 deletions(-) diff --git a/man7/socket.7 b/man7/socket.7 index db7cb8324dde..79b4f3158541 100644 --- a/man7/socket.7 +++ b/man7/socket.7 @@ -53,13 +53,6 @@ .\" SO_BPF_EXTENSIONS (3.14) .\" commit ea02f9411d9faa3553ed09ce0ec9f00ceae9885e .\"Author: Michal Sekletar -.\" SO_ATTACH_BPF (3.19) -.\" and SO_DETACH_BPF as synonym for SO_DETACH_FILTER -.\" commit 89aa075832b0da4402acebd698d0411dcc82d03e -.\"Author: Alexei Starovoitov -.\"SO_ATTACH_REUSEPORT_CBPF, SO_ATTACH_REUSEPORT_EBPF (4.5) -.\"commit 538950a1b7527a0a52ccd9337e3fcd304f027f13 -.\"Author: Craig Gallek .\" .TH SOCKET 7 2015-05-07 Linux "Linux Programmer's Manual" .SH NAME @@ -311,6 +304,80 @@ The value 0 indicates that this is not a listening socket, the value 1 indicates that this is a listening socket. This socket option is read-only. .TP +.BR SO_ATTACH_FILTER " and " SO_ATTACH_BPF +Attach a classic or extended BPF program (respectively) to the socket +for use as a filter of incoming packets. A packet will be dropped if +the filter returns zero or have its data truncated to the non-zero +length returned. If the value returned is greater or equal to the +packet's data length, the packet is allowed to proceed unmodified. + +The argument for +.BR SO_ATTACH_FILTER +is a +.I sock_fprog +structure in +.B . +.sp +.in +4n +.nf +struct sock_fprog { +unsigned short len; +struct sock_filter *filter; +}; +.fi +.in +.IP +The argument for +.BR SO_ATTACH_BPF +is a file descriptor returned by the +.BR bpf (2) +system call and must represent a program of type +.BR BPF_PROG_TYPE_SOCKET_FILTER. + +.BR SO_ATTACH_FILTER +is available in Linux 2.2. +.BR SO_ATTACH_BPF +is available in Linux 3.19. Both classic and extended BPF are +explained in the kernel source file +.I Documentation/networking/filter.txt +.TP +.BR SO_ATTACH_REUSEPORT_CBPF " and " SO_ATTACH_REUSEPORT_EBPF " (since Linux 4.5)" +For use with the +.BR SO_REUSEPORT +option, these options allow the user to define a classic or extended +BPF program (respectively) which defines how packets are assigned to +the sockets in the reuseport group. The program must return an index +between 0 and N-1 representing the socket which should receive the +packet (where N is the number of sockets in the group). If the BPF +program returns an invalid index, socket selection will fall back to +the plain +.BR SO_REUSEPORT +mechanism. + +Sockets are numbered in the order in which they are added to the group +(that is, the order of +.BR bind (2) +calls for UDP sockets or the order of +.BR listen (2) +calls for TCP sockets). New sockets added to the group will inherit +the program. When a socket is removed from the group (via +.BR close (2)) +the last socket in the group will be moved into the closed socket's +position. + +These options may be set repeatedly at any time on any single socket +in the group to replace the current BPF program used by all sockets in +the group. +.BR SO_ATTACH_REUSEPORT_CBPF +takes the same socket argument type as +.BR SO_ATTACH_FILTER +and +.BR SO_ATTACH_REUSEPORT_EBPF +takes the same socket argument type as +.BR SO_ATTACH_BPF. +UDP support for this feature is available in Linux 4.5. +TCP support for this feature is available in Linux 4.6. +.TP .B SO_BINDTODEVICE Bind this socket to a particular device like \(lqeth0\(rq, as specified in the passed interface name. @@ -368,6 +435,18 @@ Only allowed for processes with the .B CAP_NET_ADMIN capability or an effective user ID of 0. .TP +.BR SO_DETACH_FILTER " and " SO_DETACH_BPF +These options may be used to remove the BPF program attached to the +socket with either +.BR SO_ATTACH_FILTER +or +.BR SO_ATTACH_BPF. +The option value is ignored. +.BR SO_DETACH_FILTER +is available in Linux 2.2. +.BR SO_DETACH_BPF +is available in Linux 3.19. +.TP .BR SO_DOMAIN " (since Linux 2.6.32)" Retrieves the socket domain as an integer, returning a value such as .BR AF_INET6 . @@ -991,17 +1070,6 @@ where only the later program needs to set the option. Typically this difference is invisible, since, for example, a server program is designed to always set this option. -.SH BUGS -The -.B CONFIG_FILTER -socket options -.B SO_ATTACH_FILTER -and -.B SO_DETACH_FILTER -.\" FIXME Document SO_ATTACH_FILTER and SO_DETACH_FILTER -are not documented. -The suggested interface to use them is via the libpcap -library. .\" .SH AUTHORS .\" This man page was written by Andi Kleen. .SH SEE ALSO -- 2.7.0.rc3.207.g0ac5344
Re: [PATCH net] net: vrf: Remove direct access to skb->data
From: David Ahern Date: Tue, 23 Feb 2016 10:10:26 -0800 > Nik pointed that the VRF driver should be using skb_header_pointer > instead of accessing skb->data and bits beyond directly which can > be garbage. > > Cc: Nikolay Aleksandrov > Signed-off-by: David Ahern > --- > Dave: This should go into v4.4 stable as well. Applied and queued up for -stable, thanks.
Re: [PATCH RFC 3/3] net: convert tc_u32 to use the intermediate representation
On 16-02-25 09:37 AM, Pablo Neira Ayuso wrote: > This patch moves the u32 parser from the ixgbe that John has made to the > core u32. This parser has been adapted to build the intermediate > representation. > > To store the parsing information, this patch introduces a parse table > object, one per device, so we don't need to store the parsing states in > the adapter, which is the major dependency with previous patches. > > Since u32 allows rules connected via links, the u32 parser tracks this > links and then generates the intermediate representation that is passed > to the ixgbe driver. It also supports hash tables and loops. > > New drivers will only have to implement the jit translation code based > on the intermediate representation. With some extra work, I think it > should be possible to generalize the existing tc specific ndo action so > it can be used by other frontends. > > I tried to stick to John's original u32 frontend parser as much as > possible, adapting it to build the intermediate representation. > > After this change, we don't expose the tc action structure layout and > other similar frontend details to the backend anymore to the backend > anymore. I think this is good since changes in the frontend should not > need to be propagated to the 1..n drivers supporting u32 offloads. In > that sense, this helps to keep the frontend software representation > separated from low-level backend driver details. > > After this patch, it should be possible to put the tc_cls_u32_knode > structure into diet since we only need the handle (as unique id) and the > ast tree. > On ixgbe this is true but going forward I can support hash functions so you need the divisor, prio, and handle minimally. I'm not sure how to do hash tables for example in this IR yet. Might be there I'm still trying to grok the IR details. > I couldn't send any more incremental changes to update previous work > since the u32 parser and the internal representation were put together, > that why this patch is slightly large. > > Signed-off-by: Pablo Neira Ayuso > --- > drivers/net/ethernet/intel/ixgbe/ixgbe.h | 4 - > drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 216 > drivers/net/ethernet/intel/ixgbe/ixgbe_model.h | 112 > include/net/pkt_cls.h | 3 + > net/sched/cls_u32.c| 344 > + Feels like a lot of code to me when the original direct u32 to hw was reasonably small and the flower implementation is small as well. [...] > > +static int ixgbe_tcp_jit(struct net_ir_jit_ctx *ctx, > + const struct net_ir_expr *expr, > + void *data) > +{ > + struct ixgbe_filter *f = (struct ixgbe_filter *)data; > + struct net_ir_expr *right = expr->relational.right; > + struct net_ir_expr *payload; > + u32 mask = 0x; > + > + if (expr->relational.left->type == NET_IR_EXPR_BINOP) { > + payload = expr->relational.left->binop.left; > + mask = expr->relational.left->binop.right->value.data; > + } else { > + payload = expr->relational.left; > + } > + > + switch (payload->payload.offset) { I don't like this because it hardcodes the offset pieces into the driver. The reason I used a model.h header file with its structures in ixgbe is the next step is to expose the parser so the model can be updated. So that when the hardware parser changes the data structure can be changed via firmware call. The style of coding here wont work for that so we still need the model.h file and for loop. > + case offsetof(struct tcphdr, source): > + f->input->filter.formatted.src_port = right->value.data & > 0x; > + f->mask.formatted.src_port = mask & 0x; > + break; > + case offsetof(struct tcphdr, dest): > + f->input->filter.formatted.dst_port = right->value.data & > 0x; > + f->mask.formatted.dst_port = mask & 0x; > + break; > + default: > + return -EOPNOTSUPP; > + } > + return 0; > +} > + > +static struct net_ir_proto_desc ixgbe_tcp_desc = { > + .base = NET_IR_PAYLOAD_TRANSPORT_HDR, > + .protonum = IPPROTO_TCP, hmm I'm having trouble tracking down how this is used but the point of a lot of ongoing work is to not be dependent on any specific protocol values. The IPPROTO_TCP here looks suspicious why do we need this? It seems to imply I would need to define a IPPROTO_FOO if I wanted to add a new protocol. > + .jit= ixgbe_tcp_jit, > +}; > + > +static int ixgbe_ipv4_jit(struct net_ir_jit_ctx *ctx, > + const struct net_ir_expr *expr, > + void *data) > +{ > + struct ixgbe_filter *f = (struct ixgbe_filter *)data; > + struct net_ir_expr *right = expr->relational.right; > + struct net_ir_expr *payload; > + u32 mask = 0xf
Re: [patch net-next v2 0/9] Introduce devlink interface and first drivers to use it
On 25.02.2016 21:12, David Miller wrote: From: Jiri Pirko Date: Tue, 23 Feb 2016 16:51:25 +0100 There a is need for some userspace API that would allow to expose things that are not directly related to any device class like net_device of ib_device, but rather chip-wide/switch-ASIC-wide stuff. Use cases: 1) get/set of port type (Ethernet/InfiniBand) 2) setting up port splitters - split port into multiple ones and squash again, enables usage of splitter cable 3) setting up shared buffers - shared among multiple ports within one chip (work in progress) 4) configuration of switch wide properties - resources division etc - This will allow to pass configuration that is unacceptable to be passed as a module option. First patch of this set introduces a new generic Netlink based interface, called "devlink". It is similar to nl80211 model and it is heavily influenced by it, including the API definition. The devlink introduction patch implements use cases 1) and 2). Other 2 are in development atm and will be addressed by follow-ups. It is very convenient for drivers to use devlink, as you can see in other patches in this set. Counterpart for devlink is userspace tool for now called "dl". Command line interface and outputs are derived from "ip" tool so it should be easy for users to get used to it. I am very close to applying this series as-is. The clincher for me is that there is precendence in the nl80211 stuff, so obviously whatever userland infrastructure sits on top of that has found a way to deal whatever perceived shortcomings devlink has. Actually nl80211 phy interfaces aren't really managed by wpa_supplicant nor NetworkManager, but they use net_device names to discover those later on. In devlink we don't necessarily have netdev names, thus my only objection to this series is to switch to stable topology identifiers. If all that people can come up with is "device names! omg UDEV!" and "multiple tools are hard to use!" That's awesome, because it means nobody has any real substantial objection to this facility. :-) I do think this is important if a lot of devlinks are visible and they don't yet provide a net_device. One has to use e.g. pci identifiers or depend on module loading order. Especially I have no idea how this should work with devices where the module is not yet loaded (and most device drivers don't split between pci layer and net_device). This is even more complicated then the udev stuff we have nowadays. Bye, Hannes
Re: Sending short raw packets using sendmsg() broke
> Commit 9c7077622dd9174 added a check, ll_header_truncated(), which requires > that a packet transmitted using sendmsg() with PF_PACKET, SOCK_RAW must be > longer than dev->hard_header_len. > > https://github.com/torvalds/linux/commit/9c7077622dd9174 > https://github.com/torvalds/linux/blob/master/net/packet/af_packet.c#L2329 As David already pointed out, this has been revised to allow greater than or equal. Note that the behavior was already present for tpacket_snd and this patch only rationalized the two paths. > > The bug that popped up is that an application (aprx) can no longer send > short AX.25 packets using sendmsg(). Packets shorter than 77 bytes fail this > check in ll_header_truncated(). With older kernels, no problem. AX.25 (and > some other protocols) have variable-length headers (somewhere between 21 and > 77 bytes in this case). > > hard_header_len is set in drivers/net/hamradio/mkiss.c to > AX25_KISS_HEADER_LEN + AX25_MAX_HEADER_LEN + 3 which works out to be If header length is variable and can be shorter than AX25_MAX_HEADER_LEN then the check will still trigger. > (1+17+7*8+3)=77. > > https://github.com/torvalds/linux/blob/master/drivers/net/hamradio/mkiss.c#L845 > > I guessed that we could probably set hard_header_len to be the minimum > length of the packet header (AX25_KISS_HEADER_LEN + AX25_HEADER_LEN + 3) to > make things work again, but I first asked Alan Cox for an opinion, and he > says hard_header_len is set correctly to be the worst-case maximum header > length, and that the ll_header_truncated commit should be reverted instead, > since it doesn't take variable-length headers into account. hard_header_length is used in cases where we have to reserve room or check against packets that exceed frame size, so it indeed should not be changed to be the minimum header length in a variable header length scenario. In most protocols the header length is fixed, so there is no separate field for minimal header length. If this is the only such check in the kernel (and I haven't found another after a cursory inspection), then perhaps an exception should be made here for this one protocol family. > s = socket(AF_AX25, SOCK_DGRAM, 0); > > I didn't yet figure out why that works, maybe the sendto() of an AX25 > datagram does not go through that hard_header_len check. That is a different protocol family. The above check is limited to sends over packets of family PF_PACKET.
Re: header conflict introduced by change to netfilter_ipv4/ip_tables.h
On 02/04/2016 08:13 AM, Josh Boyer wrote: On Thu, Jan 7, 2016 at 2:15 PM, Mikko Rapeli wrote: On Thu, Jan 07, 2016 at 10:30:40AM -0800, Stephen Hemminger wrote: On Thu, 7 Jan 2016 07:29:50 + Mikko Rapeli wrote: On Wed, Jan 06, 2016 at 09:20:07AM -0800, Stephen Hemminger wrote: This commit breaks compilation of iproute2 with net-next. Ok, linux/if.h and libc net/if.h have overlapping defines, and this is not the only one. I saw lots of them in the core dump headers. How should we handle them? Another ifndef for IFNAMSIZ into kernel uapi headers? -Mikko Probably need to do the same thing that was done previously for these kind of conflicts. This makes make linux/if.h change to adapt to net/if.h being included before it. Ok, got it. And found include/uapi/linux/libc-compat.h. Did not know about it and was looking for solutions to these problems. But now I feel like writing a test script for mixing of kernel uapi and libc headers to find out how many other collitions are still there. Not good for the pile of over 70 patches in my branch https://github.com/torvalds/linux/compare/master...mcfrisk:headers_test_v05 Or revert your patch. I'm fine with this too. This is causing a number of build failures in Fedora rawhide now. Did anyone submit a revert or patch to fix this issue? Mikko, was there any follow-up patch to fix this? Seems like the build error is not yet resolved. Thanks, Daniel
Re: [PATCH] socket.7: Document some BPF-related socket options
On Thu, Feb 25, 2016 at 03:27:45PM -0500, Craig Gallek wrote: > From: Craig Gallek > > Document the behavior and the first kernel version for each of the > following socket options: > SO_ATTACH_FILTER > SO_ATTACH_BPF > SO_ATTACH_REUSEPORT_CBPF > SO_ATTACH_REUSEPORT_EBPF > SO_DETACH_FILTER > SO_DETACH_BPF > > Signed-off-by: Craig Gallek Thanks! Looks good to me. Acked-by: Alexei Starovoitov
Re: [PATCH 1/1] phy: marvell: Fix 88E1510 initialization
From: Clemens Gruber Date: Tue, 23 Feb 2016 20:16:58 +0100 > A bug was introduced in the merge commit b633353115e3 ("Merge > git://git.kernel.org/pub/scm/linux/kernel/git/davem/net") > The generic marvell_config_init (and therefore marvell_of_reg_init) is > not called anymore for the Marvell 88E1510 (in net-next). > > This patch calls marvell_config_init and moves the specific init > function for the 88E1510 below the marvell_config_init function to avoid > adding a function predeclaration. > > Signed-off-by: Clemens Gruber Applied, thanks for noticing and fixing this merge error!
Re: [patch net-next v2 0/9] Introduce devlink interface and first drivers to use it
Thu, Feb 25, 2016 at 09:44:43PM CET, han...@stressinduktion.org wrote: >On 25.02.2016 21:12, David Miller wrote: >>From: Jiri Pirko >>Date: Tue, 23 Feb 2016 16:51:25 +0100 >> >>>There a is need for some userspace API that would allow to expose things >>>that are not directly related to any device class like net_device of >>>ib_device, but rather chip-wide/switch-ASIC-wide stuff. >>> >>>Use cases: >>>1) get/set of port type (Ethernet/InfiniBand) >>>2) setting up port splitters - split port into multiple ones and squash >>>again, >>>enables usage of splitter cable >>>3) setting up shared buffers - shared among multiple ports within >>>one chip (work in progress) >>>4) configuration of switch wide properties - resources division etc - This >>>will >>>allow to pass configuration that is unacceptable to be passed as >>>a module option. >>> >>>First patch of this set introduces a new generic Netlink based interface, >>>called "devlink". It is similar to nl80211 model and it is heavily >>>influenced by it, including the API definition. The devlink introduction >>>patch >>>implements use cases 1) and 2). Other 2 are in development atm and will >>>be addressed by follow-ups. >>> >>>It is very convenient for drivers to use devlink, as you can see in other >>>patches in this set. >>> >>>Counterpart for devlink is userspace tool for now called "dl". Command line >>>interface and outputs are derived from "ip" tool so it should be easy >>>for users to get used to it. >> >>I am very close to applying this series as-is. >> >>The clincher for me is that there is precendence in the nl80211 stuff, >>so obviously whatever userland infrastructure sits on top of that has >>found a way to deal whatever perceived shortcomings devlink has. > >Actually nl80211 phy interfaces aren't really managed by wpa_supplicant nor >NetworkManager, but they use net_device names to discover those later on. In >devlink we don't necessarily have netdev names, thus my only objection to >this series is to switch to stable topology identifiers. Hannes, as I mentioned earlier in one of my replies, you can choose to use devlink name or pci (or other) address for your commands. So you have your stable names even before udev takes care of renaming. It's up to you as a user what handle to use.
Re: [PATCH 2/2] net: qca_spi: clear IFF_TX_SKB_SHARING
From: Stefan Wahren Date: Tue, 23 Feb 2016 19:23:24 + > ether_setup sets IFF_TX_SKB_SHARING but this is not supported by > qca_spi as it modifies the skb on xmit. > > Signed-off-by: Stefan Wahren > Fixes: 291ab06ecf67 (net: qualcomm: new Ethernet over SPI driver for QCA7000) Applied and queued up for -stable.
Re: header conflict introduced by change to netfilter_ipv4/ip_tables.h
On 01/06/16 at 09:20am, Stephen Hemminger wrote: > This commit breaks compilation of iproute2 with net-next. > > commit 1ffad83dffd675cd742286ae82dca7d746cb0da8 > Author: Mikko Rapeli > Date: Thu Oct 15 07:56:30 2015 +0200 > > netfilter: fix include files for compilation > > Add missing header dependencies and other small changes so that each file > compiles alone in userspace. > > Signed-off-by: Mikko Rapeli > Signed-off-by: Pablo Neira Ayuso > > For iproute2, a copy of kernel headers (from make install_headers) is used. > After this change. the build of x_tables.c fails because IFNAMSIZ is already > defined in net/if.h There is another issue with this commit. iptables.h included from m_ipt.c includes xtables.h which includes which is not available on a system without xtables. gcc -Wall -Wstrict-prototypes -Wmissing-prototypes -Wmissing-declarations -Wold-style-definition -Wformat=2 -O2 -I../include -DRESOLVE_HOSTNAMES -DLIBDIR=\"/usr/lib\" -DCONFDIR=\"/etc/iproute2\" -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -DHAVE_SETNS -DHAVE_ELF -DCONFIG_GACT -DCONFIG_GACT_PROB -DIPT_LIB_DIR=\"/lib/xtables\" -DYY_NO_INPUT -c -o m_ipt.o m_ipt.c In file included from ../include/iptables.h:5:0, from m_ipt.c:17: ../include/xtables.h:34:29: fatal error: xtables-version.h: No such file or directory #include
Re: [PATCH 1/2] net: qca_spi: Don't clear IFF_BROADCAST
From: Stefan Wahren Date: Tue, 23 Feb 2016 19:23:23 + > Currently qcaspi_netdev_setup accidentally clears IFF_BROADCAST. > So fix this by keeping the flags from ether_setup. > > Reported-by: Michael Heimpold > Signed-off-by: Stefan Wahren > Fixes: 291ab06ecf67 (net: qualcomm: new Ethernet over SPI driver for QCA7000) Applied and queued up for -stable.
Re: [PATCH net] Propagate MAC address changes to VLANs
From: Mike Manning Date: Tue, 23 Feb 2016 20:56:31 + > --- a/net/8021q/vlan.c > +++ b/net/8021q/vlan.c > @@ -293,15 +293,15 @@ static void vlan_sync_address(struct net > > /* vlan address was different from the old address and is equal to > * the new address */ > -if (!ether_addr_equal(vlandev->dev_addr, vlan->real_dev_addr) && > +if ((vlandev->flags & IFF_UP) && > +!ether_addr_equal(vlandev->dev_addr, vlan->real_dev_addr) && This patch is corrupted by your email client. Please read Documentation/email-clients.txt, fix your setup, and email a test patch to yourself. Only resubmit this patch to the list once you are able to successfully apply the patch in that test email.
Re: [PATCH net v4] r8169:fix "rtl_counters_cond == 1 (loop: 1000, delay: 10)" log spam.
From: Chunhao Lin Date: Wed, 24 Feb 2016 14:18:42 +0800 > There will be a log spam when there is no cable plugged. Please refer to > following links. https://bugzilla.kernel.org/show_bug.cgi?id=104351 > https://bugzilla.kernel.org/show_bug.cgi?id=107421 > > This issue is caused by runtime power management. When there is no cable > plugged, the driver will be suspend (runtime suspend) by OS and NIC will be > put into the D3 state. During this time, if OS call rtl8169_get_stats64() > to dump tally counter, because NIC is in D3 state, the register value read > by driver will return all 0xff. This will let driver think tally counter > flag is not toggled and then sends the warning message "rtl_counters_cond > == 1 (loop: 1000, delay: 10)" to kernel log. > > For fixing this issue, 1.add checking driver's pm runtime status in > rtl8169_get_stats64(). 2.dump tally counter before going runtime suspend > for counter accuracy in runtime suspend. > > Signed-off-by: Chunhao Lin Applied, thanks.
Re: [PATCHv3] net: fix bridge multicast packet checksum validation
From: Linus Lüssing Date: Wed, 24 Feb 2016 04:21:42 +0100 > We need to update the skb->csum after pulling the skb, otherwise > an unnecessary checksum (re)computation can ocure for IGMP/MLD packets > in the bridge code. Additionally this fixes the following splats for > network devices / bridge ports with support for and enabled RX checksum > offloading: > > [...] > [ 43.986968] eth0: hw csum failure > [ 43.990344] CPU: 3 PID: 0 Comm: swapper/3 Not tainted 4.4.0 #2 > [ 43.996193] Hardware name: BCM2709 > [ 43.999647] [<800204e0>] (unwind_backtrace) from [<8001cf14>] > (show_stack+0x10/0x14) > [ 44.007432] [<8001cf14>] (show_stack) from [<801ab614>] > (dump_stack+0x80/0x90) > [ 44.014695] [<801ab614>] (dump_stack) from [<802e4548>] > (__skb_checksum_complete+0x6c/0xac) > [ 44.023090] [<802e4548>] (__skb_checksum_complete) from [<803a055c>] > (ipv6_mc_validate_checksum+0x104/0x178) > [ 44.032959] [<803a055c>] (ipv6_mc_validate_checksum) from [<802e111c>] > (skb_checksum_trimmed+0x130/0x188) > [ 44.042565] [<802e111c>] (skb_checksum_trimmed) from [<803a06e8>] > (ipv6_mc_check_mld+0x118/0x338) > [ 44.051501] [<803a06e8>] (ipv6_mc_check_mld) from [<803b2c98>] > (br_multicast_rcv+0x5dc/0xd00) > [ 44.060077] [<803b2c98>] (br_multicast_rcv) from [<803aa510>] > (br_handle_frame_finish+0xac/0x51c) > [...] > > Fixes: 9afd85c9e455 ("net: Export IGMP/MLD message validation code") > Reported-by: Álvaro Fernández Rojas > Signed-off-by: Linus Lüssing Applied and queued up for -stable, thanks!
Re: [v2, 0/3] Add PTP support for ls1021a platform
From: Yangbo Lu Date: Wed, 24 Feb 2016 17:26:53 +0800 > This patchset is to enable ptp support for ls1021a platform. The endianness > issue in gianfar driver and gianfar ptp driver must be fixed, and a 1588 > timer node must be added into dts. > > Changes for v2: > - Modified commit message > - Added more reviewers Series applied to net-next, thanks.
Re: [PATCH] net: thunderx: Fix for Qset error due to CQ full
From: sunil.kovv...@gmail.com Date: Wed, 24 Feb 2016 16:40:50 +0530 > From: Sunil Goutham > > On Thunderx pass 1.x and pass2 due to a HW errata default CQ > DROP_LEVEL of 0x80 is not sufficient to avoid CQ_WR_FULL Qset > error when packets are being received at >20Mpps resulting in > complete stall of packet reception. > > This patch will configure it to 0x100 which is what is expected > by HW on Thunderx. On future passes of thunderx and other chips > HW default/reset value will be 0x100 or higher hence not overwritten. > > Signed-off-by: Jerin Jacob > Signed-off-by: Sunil Goutham Applied, thank you.
Re: [PATCH net 3/7] net/mlx5e: Fix soft lockup when HW Timestamping is enabled
From: Saeed Mahameed Date: Wed, 24 Feb 2016 13:18:49 +0200 > From: Eran Ben Elisha > > Readers/Writers lock for SW timecounter was acquired without disabling > interrupts on local CPU. > > The problematic scenario: > * HW timestamping is enabled > * Timestamp overflow periodic service task is running on local CPU and > holding write_lock for SW timecounter > * Completion arrives, triggers interrupt for local CPU. > Interrupt routine calls napi_schedule(), which triggers rx/tx > skb process. > An attempt to read SW timecounter using read_lock is done, which is > already locked by a writer on the same CPU and cause soft lockup. > > Add irqsave/irqrestore for when using the readers/writers lock. > > Fixes: ef9814deafd0 ('net/mlx5e: Add HW timestamping (TS) support') > Signed-off-by: Eran Ben Elisha > Signed-off-by: Saeed Mahameed I'd like to suggest a slight modification to this fix. The canonical way to deal with this issue is to use IRQ disabling for the write lock side, but not for the read lock side. You only need to prevent a read lock via interrupt from nesting on the same cpu where you took the write lock. So you don't need the expensive IRQ disabling for the read lock side, which is the more important path performance wise. Thanks.
Re: [net-next PATCH 0/5] net_sched: Add support for IFE action
On 02/25/2016 01:23 PM, Jamal Hadi Salim wrote: On 16-02-24 12:48 PM, Daniel Borkmann wrote: On 02/24/2016 01:49 PM, Jamal Hadi Salim wrote: [...] Drivers do set the hash. My use case is slightly different. I have a NIC which has an embedded cavium processor. This thing strips off the TLV and uses the hash to select the host MSI. Only thing we dont use at the moment is queue_mapping. Ok, but the example says ingress qdisc. ;) I presume the driver for the NIC and the offloading parts are non-public? :/ Well, it is not exactly mellanox or intel - but I am sure someone would be willing to sell that NIC. So, without them, placing this on ingress qdisc doesn't seem much useful wrt the skb hash example, and most people only have the software part (for ingress I mean) available. Do you want me to take skbbhash out? I just want to get this set in then worry about adding and modifying. Well, if the NIC driver and offloading parts for ife are not available (or cannot be made available) in the upstream kernel tree, then you have to assume that everyone else can _only_ use this with the existing tc facilities in the kernel. And as such, the whole set needs to be evaluated, I think this is nothing new. ;-) That is why I asked for a "typical setup" and use-case earlier. And if it doesn't have any obvious ones in upstream context without the missing pieces, then the code might linger around unused by anyone. So taking out these parts would be highly preferred (unless there's a _good_ reason not to). Thanks, Daniel
[PATCH net-next] hv_netvsc: add ethtool support for set and get of settings
This patch allows the user to set and retrieve speed and duplex of the hv_netvsc device via ethtool. Example: $ ethtool eth0 Settings for eth0: ... Speed: Unknown! Duplex: Unknown! (255) ... $ ethtool -s eth0 speed 1000 duplex full $ ethtool eth0 Settings for eth0: ... Speed: 1000Mb/s Duplex: Full ... This is based on patches by Roopa Prabhu and Nikolay Aleksandrov. Signed-off-by: Simon Xiao --- drivers/net/hyperv/hyperv_net.h | 4 +++ drivers/net/hyperv/netvsc_drv.c | 56 + 2 files changed, 60 insertions(+) diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h index fcb92c0..b4c6878 100644 --- a/drivers/net/hyperv/hyperv_net.h +++ b/drivers/net/hyperv/hyperv_net.h @@ -658,6 +658,10 @@ struct net_device_context { struct netvsc_stats __percpu *tx_stats; struct netvsc_stats __percpu *rx_stats; + + /* Ethtool settings */ + u8 duplex; + u32 speed; }; /* Per netvsc device */ diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c index 202e2b1..e703b9a 100644 --- a/drivers/net/hyperv/netvsc_drv.c +++ b/drivers/net/hyperv/netvsc_drv.c @@ -799,6 +799,58 @@ static int netvsc_set_channels(struct net_device *net, goto do_set; } +static bool netvsc_validate_ethtool_ss_cmd(const struct ethtool_cmd *cmd) +{ + struct ethtool_cmd diff1 = *cmd; + struct ethtool_cmd diff2 = {}; + + ethtool_cmd_speed_set(&diff1, 0); + diff1.duplex = 0; + /* advertising and cmd are usually set */ + diff1.advertising = 0; + diff1.cmd = 0; + /* We set port to PORT_OTHER */ + diff2.port = PORT_OTHER; + + return !memcmp(&diff1, &diff2, sizeof(diff1)); +} + +static void netvsc_init_settings(struct net_device *dev) +{ + struct net_device_context *ndc = netdev_priv(dev); + + ndc->speed = SPEED_UNKNOWN; + ndc->duplex = DUPLEX_UNKNOWN; +} + +static int netvsc_get_settings(struct net_device *dev, struct ethtool_cmd *cmd) +{ + struct net_device_context *ndc = netdev_priv(dev); + + ethtool_cmd_speed_set(cmd, ndc->speed); + cmd->duplex = ndc->duplex; + cmd->port = PORT_OTHER; + + return 0; +} + +static int netvsc_set_settings(struct net_device *dev, struct ethtool_cmd *cmd) +{ + struct net_device_context *ndc = netdev_priv(dev); + u32 speed; + + speed = ethtool_cmd_speed(cmd); + if (!ethtool_validate_speed(speed) || + !ethtool_validate_duplex(cmd->duplex) || + !netvsc_validate_ethtool_ss_cmd(cmd)) + return -EINVAL; + + ndc->speed = speed; + ndc->duplex = cmd->duplex; + + return 0; +} + static int netvsc_change_mtu(struct net_device *ndev, int mtu) { struct net_device_context *ndevctx = netdev_priv(ndev); @@ -923,6 +975,8 @@ static const struct ethtool_ops ethtool_ops = { .get_channels = netvsc_get_channels, .set_channels = netvsc_set_channels, .get_ts_info= ethtool_op_get_ts_info, + .get_settings = netvsc_get_settings, + .set_settings = netvsc_set_settings, }; static const struct net_device_ops device_ops = { @@ -1112,6 +1166,8 @@ static int netvsc_probe(struct hv_device *dev, netif_set_real_num_tx_queues(net, nvdev->num_chn); netif_set_real_num_rx_queues(net, nvdev->num_chn); + netvsc_init_settings(net); + ret = register_netdev(net); if (ret != 0) { pr_err("Unable to register netdev.\n"); -- 2.5.0
Re: [net-next PATCH v2 1/5] introduce IFE action
On 02/25/2016 01:20 PM, Jamal Hadi Salim wrote: On 16-02-24 12:37 PM, Daniel Borkmann wrote: On 02/23/2016 01:49 PM, Jamal Hadi Salim wrote: From: Jamal Hadi Salim [...] +static const struct nla_policy ife_policy[TCA_IFE_MAX + 1] = { +[TCA_IFE_PARMS] = {.len = sizeof(struct tc_ife)}, +[TCA_IFE_DMAC] = {.type = NLA_BINARY,.len = ETH_ALEN}, +[TCA_IFE_SMAC] = {.type = NLA_BINARY,.len = ETH_ALEN}, This is buggy btw ... I am sure i cutnpasted that from somewhere. Thanks for catching it; I will remove NLA_BINARY ref. Yeah, NLA_BINARY seems to be a bit of a misleading name. We should probably audit, if there are more such users already in the tree. [...] Maybe try to make this lockless in the fast path? Otherwise placing this on ingress / egress (f.e. clsact) doesn't really scale. Let me think about it. Likely it will be subsequent patches - I just want to get this set out first. Yes, I mean one of the key motivation was "[...] to horizontally scale packet processing at scope of a chasis or rack [...]". So for people who don't have that NIC with embedded Cavium processor, they might already hit scalability issues for encode/decode right there. Thanks again, Daniel
Re: [patch net-next v2 0/9] Introduce devlink interface and first drivers to use it
On Thu, Feb 25, 2016 at 03:12:47PM -0500, David Miller wrote: > From: Jiri Pirko > Date: Tue, 23 Feb 2016 16:51:25 +0100 > > > There a is need for some userspace API that would allow to expose things > > that are not directly related to any device class like net_device of > > ib_device, but rather chip-wide/switch-ASIC-wide stuff. > > > > Use cases: > > 1) get/set of port type (Ethernet/InfiniBand) > > 2) setting up port splitters - split port into multiple ones and squash > > again, > >enables usage of splitter cable > > 3) setting up shared buffers - shared among multiple ports within > >one chip (work in progress) > > 4) configuration of switch wide properties - resources division etc - This > > will > >allow to pass configuration that is unacceptable to be passed as > >a module option. > > > > First patch of this set introduces a new generic Netlink based interface, > > called "devlink". It is similar to nl80211 model and it is heavily > > influenced by it, including the API definition. The devlink introduction > > patch > > implements use cases 1) and 2). Other 2 are in development atm and will > > be addressed by follow-ups. > > > > It is very convenient for drivers to use devlink, as you can see in other > > patches in this set. > > > > Counterpart for devlink is userspace tool for now called "dl". Command line > > interface and outputs are derived from "ip" tool so it should be easy > > for users to get used to it. > > I am very close to applying this series as-is. > > The clincher for me is that there is precendence in the nl80211 stuff, > so obviously whatever userland infrastructure sits on top of that has > found a way to deal whatever perceived shortcomings devlink has. > > If all that people can come up with is "device names! omg UDEV!" and > "multiple tools are hard to use!" That's awesome, because it means > nobody has any real substantial objection to this facility. :-) Generally speaking I had no issue with the patch after being *ahem* corrected to realize that the set operations only applied to the devlink devices not specifically to netdevs. I find this interface useful for some other items we have discussed in the past (resource limits for routes, etc), so I'm happy to see some interface for general chip maintenance included.
Re: [PATCH 2/3] forcedeth: Use setup_timer()
From: Amitoj Kaur Chawla Date: Wed, 24 Feb 2016 19:28:01 +0530 > Convert a call to init_timer and accompanying intializations of > the timer's data and function fields to a call to setup_timer. > > The Coccinelle semantic patch that fixes this problem is > as follows: > > // > @@ > expression t,f,d; > @@ > > -init_timer(&t); > +setup_timer(&t,f,d); > -t.data = d; > -t.function = f; > // > > Signed-off-by: Amitoj Kaur Chawla Applied.
Re: [PATCH 1/3] net: tulip: Use setup_timer()
From: Amitoj Kaur Chawla Date: Wed, 24 Feb 2016 19:27:49 +0530 > Convert a call to init_timer and accompanying intializations of > the timer's data and function fields to a call to setup_timer. > > The Coccinelle semantic patch that fixes this problem is > as follows: > > // > @@ > expression t,f,d; > @@ > > -init_timer(&t); > +setup_timer(&t,f,d); > -t.data = d; > -t.function = f; > // > > Signed-off-by: Amitoj Kaur Chawla Applied.
Re: [PATCH 3/3] 3c59x: Use setup_timer()
From: Amitoj Kaur Chawla Date: Wed, 24 Feb 2016 19:28:19 +0530 > Convert a call to init_timer and accompanying intializations of > the timer's data and function fields to a call to setup_timer. > > The Coccinelle semantic patch that fixes this problem is > as follows: > > // > @@ > expression t,f,d; > @@ > > -init_timer(&t); > +setup_timer(&t,f,d); > ... > -t.data = d; > -t.function = f; > // > > Signed-off-by: Amitoj Kaur Chawla Applied.
Re: [PATCH] netxen: Use kobj_to_dev()
From: Amitoj Kaur Chawla Date: Wed, 24 Feb 2016 20:09:38 +0530 > Introduce the use of kobj_to_dev() helper function instead of open > coding it with container_of() > > The Coccinelle semantic patch used to make this change is as follows: > > // > @@ > expression a; > symbol kobj; > @@ > - container_of(a, struct device, kobj) > + kobj_to_dev(a) > // > > Signed-off-by: Amitoj Kaur Chawla Applied.
Re: [patch net-next v2 0/9] Introduce devlink interface and first drivers to use it
On 25.02.2016 22:12, Jiri Pirko wrote: Thu, Feb 25, 2016 at 09:44:43PM CET, han...@stressinduktion.org wrote: On 25.02.2016 21:12, David Miller wrote: From: Jiri Pirko Date: Tue, 23 Feb 2016 16:51:25 +0100 There a is need for some userspace API that would allow to expose things that are not directly related to any device class like net_device of ib_device, but rather chip-wide/switch-ASIC-wide stuff. Use cases: 1) get/set of port type (Ethernet/InfiniBand) 2) setting up port splitters - split port into multiple ones and squash again, enables usage of splitter cable 3) setting up shared buffers - shared among multiple ports within one chip (work in progress) 4) configuration of switch wide properties - resources division etc - This will allow to pass configuration that is unacceptable to be passed as a module option. First patch of this set introduces a new generic Netlink based interface, called "devlink". It is similar to nl80211 model and it is heavily influenced by it, including the API definition. The devlink introduction patch implements use cases 1) and 2). Other 2 are in development atm and will be addressed by follow-ups. It is very convenient for drivers to use devlink, as you can see in other patches in this set. Counterpart for devlink is userspace tool for now called "dl". Command line interface and outputs are derived from "ip" tool so it should be easy for users to get used to it. I am very close to applying this series as-is. The clincher for me is that there is precendence in the nl80211 stuff, so obviously whatever userland infrastructure sits on top of that has found a way to deal whatever perceived shortcomings devlink has. Actually nl80211 phy interfaces aren't really managed by wpa_supplicant nor NetworkManager, but they use net_device names to discover those later on. In devlink we don't necessarily have netdev names, thus my only objection to this series is to switch to stable topology identifiers. Hannes, as I mentioned earlier in one of my replies, you can choose to use devlink name or pci (or other) address for your commands. So you have your stable names even before udev takes care of renaming. It's up to you as a user what handle to use. I understood that part from the beginning. In my opinion we should simply keep APIs simple and clean, reduced to the minimum and let user space build upon this. If an entity in the kernel doesn't need a name and/or a naming database, I would not bother implementing it. Not everyone is a kernel developer who uses Linux and can infer the problems from dl --help. I bet most users will use names at some point in time and they simply can't match even with the udev quirks in place when setting up netbooting or configuration must happen from the currently provided initramfs. This is my point. If names are fragile to use simply don't provide it if not necessary. devlink is an interface of its own and the names won't be referenced by other subsystems like iptables -i or iproute. So why bother with names in the kernel? On the rest with netlink etc. I agree already. I just want to eliminate possible mistakes by users. Hope that makes my point more clear, Hannes
Re: [net-next PATCH 3/4] net: sched: cls_u32 add bit to specify software only rules
On 16-02-25 04:56 AM, Jamal Hadi Salim wrote: > On 16-02-24 11:04 PM, John Fastabend wrote: >> On 16-02-24 05:31 AM, Jamal Hadi Salim wrote: > >> I think this is absolutely necessary not only for performance of >> reporting the rules back to software but if we don't do it generically >> the driver will have to do it anyways because doing the inverse >> transformation from hw implementation to u32 is really tricky and in >> fact with hnodes and knodes there are multiple cls_u32 "programs" that >> functionally are the same so we have no way to return what the user >> actually programmed without it. Further eBPF (the next classifier I'm >> working on) is even worse in this regard. > > Ok, I guess there are multiple use cases for it ;-> > Yes, with ebpf it will be worse because data and instructions are > inter- mingled (and our interest is in data only). Note: > Over the years this has been a big struggle for human > friendliness. I thought we didnt care about humans (as in automation) > but you are saying this will affect machines too ;-> We cant allow > that ;-> > > Note: You could decode u32 descriptions but it is an involved effort. > Example, see this feature in tc: > --- > jhs@jhs1 tc -pretty filter ls dev $ETH parent : protocol ip > filter pref 11 u32 > filter pref 11 u32 fh 800: ht divisor 1 > filter pref 11 u32 fh 800::800 order 2048 key ht 800 bkt 0 flowid 1:1 > match IP src 10.0.0.130/32 > action order 1: gact action drop > random type none pass val 0 > index 1 ref 1 bind 1 > decoding that is not a problem. The ixgbe driver code already applied can decode that without much trouble. The thing I want to avoid is requiring my driver to do the inverse translation because although it is possible its entirely unnecessary. To do the above example without a software cache for example means I read out row 2048 of a hardware table then you get a bunch of bits. From those bits I consult what fields are being loaded into the table in the packet processing pipeline. I learn its the src_ip fields then I have the value/mask and can unwind it. Finally if I collapsed some hash tables onto this hardware table I have to do the inverse of my collapsing scheme. The ixgbe one is sort of simple just because I only have one table in hardware but with multiple tables its a bit more difficult. Finally I've unwound the thing and can print something back out of 'tc' but it seems easier to just cache the hardware rules somewhere. Maybe other driver/hardware will have a different opinion though depending on how much your firmware can store and how ambitious you are. Personally I don't see any need for the above code. > See that "match" field reading in anglais? It requires more and more > additions of pretty printers that translate back. > > What about adding some tag to allow for easy "babel translation"? > not sure what this would be... >> You can see my solution to >> this "load in hardware" filter list in patch 4/4. See Jiri's comment >> also on that and see if you agree. > > Ok, will do. > > cheers, > jamal
Re: [net-next v2 00/15][pull request] 1GbE Intel Wired LAN Driver Updates 2016-02-24
On Thu, 2016-02-25 at 09:45 +0200, Or Gerlitz wrote: > On Thu, Feb 25, 2016 at 4:14 AM, Jeff Kirsher > wrote: > > v2: Dropped patches 6-10 in the original series. Patch 6-7 added > support > > for character device for AVB and based on community feedback, > we do not > > want to do this. Patches 8-10 provided fixes to the > problematic code > > added in patches 6 & 7. So all of them must go! > > Jeff, > > Did you had special reason not to squash patches 8-10 into 6-7 in the > 1st place? Well is all water under the bridge now, but I had thought of just merging Julia's patches into Gangfeng's original series, the only reason I did not was that the fixes were not by the original author, so Julia would have lost the recognition for fixes. Had the the original series not compiled or was functionally broken, I would have required Gangfeng to re-spin the original series. signature.asc Description: This is a digitally signed message part
Re: [PATCH net-next 1/1] tipc: fix crash during node removal
From: Jon Maloy Date: Wed, 24 Feb 2016 11:10:48 -0500 > When the TIPC module is unloaded, we have identified a race condition > that allows a node reference counter to go to zero and the node instance > being freed before the node timer is finished with accessing it. This > leads to occasional crashes, especially in multi-namespace environments. > > The scenario goes as follows: > > CPU0:(node_stop) CPU1:(node_timeout) // ref == 2 > > 1: if(!mod_timer()) > 2: if (del_timer()) > 3: tipc_node_put()// ref -> 1 > 4: tipc_node_put() // ref -> 0 > 5: kfree_rcu(node); > 6: tipc_node_get(node) > 7: // BOOM! > > We now clean up this functionality as follows: > > 1) We remove the node pointer from the node lookup table before we >attempt deactivating the timer. This way, we reduce the risk that >tipc_node_find() may obtain a valid pointer to an instance marked >for deletion; a harmless but undesirable situation. > > 2) We use del_timer_sync() instead of del_timer() to safely deactivate >the node timer without any risk that it might be reactivated by the >timeout handler. There is no risk of deadlock here, since the two >functions never touch the same spinlocks. > > 3: We remove a pointless tipc_node_get() + tipc_node_put() from the >timeout handler. > > Reported-by: Zhijiang Hu > Acked-by: Ying Xue > Signed-off-by: Jon Maloy Applied.
Re: [PATCH net-next 1/1] tipc: eliminate risk of finding to-be-deleted node instance
From: Jon Maloy Date: Wed, 24 Feb 2016 11:00:19 -0500 > Although we have never seen it happen, we have identified the > following problematic scenario when nodes are stopped and deleted: > > CPU0:CPU1: > > tipc_node_xxx() //ref == 1 >tipc_node_put()//ref -> 0 > tipc_node_find() // node still in table >tipc_node_delete() > list_del_rcu(n. list) > tipc_node_get() //ref -> 1, bad > kfree_rcu() > > tipc_node_put() //ref to 0 again. > kfree_rcu() // BOOM! > > We fix this by introducing use of the conditional kref_get_if_not_zero() > instead of kref_get() in the function tipc_node_find(). This eliminates > any risk of post-mortem access. > > Reported-by: Zhijiang Hu > Acked-by: Ying Xue > Signed-off-by: Jon Maloy Applied.
Re: [PATCH net-next] tipc: fix null deref crash in compat config path
From: Florian Westphal Date: Wed, 24 Feb 2016 17:20:17 +0100 > msg.dst_sk needs to be set up with a valid socket because some callbacks > later derive the netns from it. > > Fixes: 263ea09084d172d ("Revert "genl: Add genlmsg_new_unicast() for unicast > message allocation") > Reported-by: Jon Maloy > Bisected-by: Jon Maloy > Signed-off-by: Florian Westphal > Acked-by Jon Maloy Applied.
Re: [net-next PATCH v2 1/5] introduce IFE action
On 16-02-25 01:46 PM, Daniel Borkmann wrote: > On 02/25/2016 01:20 PM, Jamal Hadi Salim wrote: >> On 16-02-24 12:37 PM, Daniel Borkmann wrote: >>> On 02/23/2016 01:49 PM, Jamal Hadi Salim wrote: From: Jamal Hadi Salim >>> [...] +static const struct nla_policy ife_policy[TCA_IFE_MAX + 1] = { +[TCA_IFE_PARMS] = {.len = sizeof(struct tc_ife)}, +[TCA_IFE_DMAC] = {.type = NLA_BINARY,.len = ETH_ALEN}, +[TCA_IFE_SMAC] = {.type = NLA_BINARY,.len = ETH_ALEN}, >>> >>> This is buggy btw ... >> >> I am sure i cutnpasted that from somewhere. Thanks for catching >> it; I will remove NLA_BINARY ref. > > Yeah, NLA_BINARY seems to be a bit of a misleading name. We should > probably audit, if there are more such users already in the tree. > At some point in the past (maybe a year ago?) I went through and fixed a handful of these but yeah it seems to be a common error. > [...] >>> Maybe try to make this lockless in the fast path? Otherwise placing >>> this on ingress / egress (f.e. clsact) doesn't really scale. >> >> Let me think about it. Likely it will be subsequent patches - I just >> want to get this set out first. > > Yes, I mean one of the key motivation was "[...] to horizontally scale > packet processing at scope of a chasis or rack [...]". So for people > who don't have that NIC with embedded Cavium processor, they might > already hit scalability issues for encode/decode right there. > > Thanks again, > Daniel
Re: [Patch] rose_route_frame() NULL pointer dereference kernel panic
From: f6bvp Date: Wed, 24 Feb 2016 17:53:11 +0100 > @@ -863,6 +863,11 @@ int rose_route_frame(struct sk_buff *skb, ax25_cb > *ax25) First of all this patch was corrupted by your email client. > int res = 0; > char buf[11]; > > + if (ax25 == NULL) { > + printk("Null ax25 destination !\n"); > + return res; > + } Second of all, this log message is not appropriate, especially given that rose_xmit() calls rose_rout_frame() with an explicit NULL second argument.
Re: [PATCH/RFC v4 net-next] ravb: Add dma queue interrupt support
On 02/21/2016 10:16 PM, Sergei Shtylyov wrote: [...] From: Kazuya Mizuguchi This patch supports the following interrupts. - One interrupt for multiple (descriptor, error, management) - One interrupt for emac - Four interrupts for dma queue (best effort rx/tx, network control rx/tx) This patch improve efficiency of the interrupt handler by adding the interrupt handler corresponding to each interrupt source described above. Additionally, it reduces the number of times of the access to EthernetAVB IF. Signed-off-by: Kazuya Mizuguchi Signed-off-by: Yoshihiro Kaneko [...] diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c index ac43ed9..076f25f 100644 --- a/drivers/net/ethernet/renesas/ravb_main.c +++ b/drivers/net/ethernet/renesas/ravb_main.c [...] + struct net_device *ndev, struct device *dev, + const char *ch) +{ + char *name; + int error; + + name = devm_kasprintf(dev, GFP_KERNEL, "%s:%s", ndev->name, ch); + error = request_irq(irq, handler, IRQF_SHARED, name, ndev); Not sure if we need IRQF_SHARED on those IRQs, they're not really shareable... I don't know whether this causes something bad. I think this controller is supporting a shared IRQ. Based on the high-level trigger, I'd rather suspect not. Anyway, all the SoC IRQs are dedicated to a certain (single) source. I don't want to change that if there is no fatal issue in the use of IRQF_SHARED. However, I will remove the flag if it is simple waste... It's not a waste but it just shouldn't be needed. Actually for RX/TX DMA you're routing 2 interrupts to the same handler, so it's needed, sorry. Scratch that, it's not multiple handlers, it's multiple IRQs... [...] Thanks, kaneko MBR, Sergei
[PATCH] mt7601u: do not free dma_buf when ivp allocation fails
From: Colin Ian King If the allocation of ivp fails the error handling attempts to free an uninitialized dma_buf; this data structure just contains garbage on the stack, so the freeing will cause issues when the urb, buf and dma fields are cleaned. Fix this by handling the ivp check and Signed-off-by: Colin Ian King --- drivers/net/wireless/mediatek/mt7601u/mcu.c | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/net/wireless/mediatek/mt7601u/mcu.c b/drivers/net/wireless/mediatek/mt7601u/mcu.c index fbb1986..06ac657 100644 --- a/drivers/net/wireless/mediatek/mt7601u/mcu.c +++ b/drivers/net/wireless/mediatek/mt7601u/mcu.c @@ -359,13 +359,13 @@ mt7601u_upload_firmware(struct mt7601u_dev *dev, const struct mt76_fw *fw) struct mt7601u_dma_buf dma_buf; void *ivb; u32 ilm_len, dlm_len; - int i, ret; + int i, ret = -ENOMEM; ivb = kmemdup(fw->ivb, sizeof(fw->ivb), GFP_KERNEL); - if (!ivb || mt7601u_usb_alloc_buf(dev, MCU_FW_URB_SIZE, &dma_buf)) { - ret = -ENOMEM; + if (!ivb) + goto error_ivb; + if (mt7601u_usb_alloc_buf(dev, MCU_FW_URB_SIZE, &dma_buf)) goto error; - } ilm_len = le32_to_cpu(fw->hdr.ilm_len) - sizeof(fw->ivb); dev_dbg(dev->dev, "loading FW - ILM %u + IVB %zu\n", @@ -396,8 +396,9 @@ mt7601u_upload_firmware(struct mt7601u_dev *dev, const struct mt76_fw *fw) dev_dbg(dev->dev, "Firmware running!\n"); error: - kfree(ivb); mt7601u_usb_free_buf(dev, &dma_buf); +error_ivb: + kfree(ivb); return ret; } -- 2.7.0
Re: [PATCH net-next 0/6] qed*: Driver updates
From: Yuval Mintz Date: Wed, 24 Feb 2016 16:52:44 +0200 > Usually I try to provide a sensible description of the patch set even if > it lacks a general 'motif', but this simply contains several small, > unrelated and self-explenatory tweaks and additions. Series applied, however I really wonder about how you stop and start the TX queues on carrier status change. The qdisc layer does that for you already, and prevents any TX from occuring when the carrier is off. So please explore removing that seemingly needless code. Thanks.
Re: [net-next PATCH 0/5] net_sched: Add support for IFE action
On 16-02-25 04:34 PM, Daniel Borkmann wrote: On 02/25/2016 01:23 PM, Jamal Hadi Salim wrote: On 16-02-24 12:48 PM, Daniel Borkmann wrote: On 02/24/2016 01:49 PM, Jamal Hadi Salim wrote: [...] Drivers do set the hash. My use case is slightly different. I have a NIC which has an embedded cavium processor. This thing strips off the TLV and uses the hash to select the host MSI. Only thing we dont use at the moment is queue_mapping. Ok, but the example says ingress qdisc. ;) I presume the driver for the NIC and the offloading parts are non-public? :/ Well, it is not exactly mellanox or intel - but I am sure someone would be willing to sell that NIC. So, without them, placing this on ingress qdisc doesn't seem much useful wrt the skb hash example, and most people only have the software part (for ingress I mean) available. Do you want me to take skbbhash out? I just want to get this set in then worry about adding and modifying. Well, if the NIC driver and offloading parts for ife are not available (or cannot be made available) If you are willing to pay for one i can have one sold to you. Note: There is no dependency on such a NIC. You asked for an example of how one would use skbhash and i pointed it out to you. Infact nothing in the commit notes pointed to any NIC. in the upstream kernel tree, then you have to assume that everyone else can _only_ use this with the existing tc facilities in the kernel. And as such, the whole set needs to be evaluated, I think this is nothing new. ;-) Seriously this is getting to a ridiculous level now. I gave a talk - which you attended. I wrote a paper which you may have at minimal glanced at. I have had discussions with you on this very subject. And as soon as i posted the patches your statements led from you needing a good use case to why not use a netdev and why i have all these things that are not very useful. None of which came up before. For someone who works on ebpf - where there is plenty of code that noone gets to see you are making rather bold statements. That is why I asked for a "typical setup" and use-case earlier. And if it doesn't have any obvious ones in upstream context without the missing pieces, then the code might linger around unused by anyone. So taking out these parts would be highly preferred (unless there's a _good_ reason not to). So is there anything that is useful in your view? cheers, jamal
Re: [net-next PATCH v2 1/5] introduce IFE action
On 16-02-25 04:46 PM, Daniel Borkmann wrote: On 02/25/2016 01:20 PM, Jamal Hadi Salim wrote: Let me think about it. Likely it will be subsequent patches - I just want to get this set out first. Yes, I mean one of the key motivation was "[...] to horizontally scale packet processing at scope of a chasis or rack [...]". By adding more processing points horizontally. Please read the paper; I think we are getting to a point where this discussion is no longer productive. So for people who don't have that NIC with embedded Cavium processor, they might already hit scalability issues for encode/decode right there. A lock on a policy that already matches is not a serious scaling problem that you need to offload to an embedded NIC. Do note the point here is to scale by adding more machines. And this shit is deployed and has proven it does scale. cheers, jamal
[Patch net-next v3 0/4] net_sched: update backlog for hierarchical qdisc's
For hierarchical qdisc like HTB, we currently only update its qlen but leave its backlog as zero: qdisc htb 1: dev eth0 root refcnt 2 r2q 10 default 1 direct_packets_stat 0 ver 3.17 Sent 172680457356 bytes 222469449 pkt (dropped 0, overlimits 123575834 requeues 0) backlog 0b 72p requeues 0 This patchset makes backlog as accurate as qlen. --- v3: rebase and fix the n==0 case for qdisc_tree_reduce_backlog() v2: rebase and update changelog, not code change Cong Wang (4): net_sched: introduce qdisc_replace() helper net_sched: update hierarchical backlog too sch_htb: update backlog as well sch_dsmark: update backlog as well include/net/codel.h | 4 include/net/sch_generic.h | 20 +++- net/sched/sch_api.c | 8 +--- net/sched/sch_cbq.c | 12 net/sched/sch_choke.c | 6 -- net/sched/sch_codel.c | 10 ++ net/sched/sch_drr.c | 9 +++-- net/sched/sch_dsmark.c| 11 --- net/sched/sch_fq.c| 4 +++- net/sched/sch_fq_codel.c | 17 - net/sched/sch_hfsc.c | 9 +++-- net/sched/sch_hhf.c | 10 +++--- net/sched/sch_htb.c | 24 +++- net/sched/sch_multiq.c| 16 ++-- net/sched/sch_netem.c | 13 +++-- net/sched/sch_pie.c | 5 +++-- net/sched/sch_prio.c | 15 +-- net/sched/sch_qfq.c | 9 +++-- net/sched/sch_red.c | 10 +++--- net/sched/sch_sfb.c | 10 +++--- net/sched/sch_sfq.c | 16 +--- net/sched/sch_tbf.c | 15 ++- 22 files changed, 126 insertions(+), 127 deletions(-) -- 2.1.0
[Patch net-next v3 3/4] sch_htb: update backlog as well
We saw qlen!=0 but backlog==0 on our production machine: qdisc htb 1: dev eth0 root refcnt 2 r2q 10 default 1 direct_packets_stat 0 ver 3.17 Sent 172680457356 bytes 222469449 pkt (dropped 0, overlimits 123575834 requeues 0) backlog 0b 72p requeues 0 The problem is we only count qlen for HTB qdisc but not backlog. We need to update backlog too when we update qlen, so that we can at least know the average packet length. Cc: Jamal Hadi Salim Acked-by: Jamal Hadi Salim Signed-off-by: Cong Wang --- net/sched/sch_htb.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c index 846a7f9..87b02ed3 100644 --- a/net/sched/sch_htb.c +++ b/net/sched/sch_htb.c @@ -600,6 +600,7 @@ static int htb_enqueue(struct sk_buff *skb, struct Qdisc *sch) htb_activate(q, cl); } + qdisc_qstats_backlog_inc(sch, skb); sch->q.qlen++; return NET_XMIT_SUCCESS; } @@ -889,6 +890,7 @@ static struct sk_buff *htb_dequeue(struct Qdisc *sch) ok: qdisc_bstats_update(sch, skb); qdisc_unthrottled(sch); + qdisc_qstats_backlog_dec(sch, skb); sch->q.qlen--; return skb; } @@ -955,6 +957,7 @@ static unsigned int htb_drop(struct Qdisc *sch) unsigned int len; if (cl->un.leaf.q->ops->drop && (len = cl->un.leaf.q->ops->drop(cl->un.leaf.q))) { + sch->qstats.backlog -= len; sch->q.qlen--; if (!cl->un.leaf.q->q.qlen) htb_deactivate(q, cl); @@ -984,12 +987,12 @@ static void htb_reset(struct Qdisc *sch) } cl->prio_activity = 0; cl->cmode = HTB_CAN_SEND; - } } qdisc_watchdog_cancel(&q->watchdog); __skb_queue_purge(&q->direct_queue); sch->q.qlen = 0; + sch->qstats.backlog = 0; memset(q->hlevel, 0, sizeof(q->hlevel)); memset(q->row_mask, 0, sizeof(q->row_mask)); for (i = 0; i < TC_HTB_NUMPRIO; i++) -- 2.1.0
[Patch net-next v3 4/4] sch_dsmark: update backlog as well
Similarly, we need to update backlog too when we update qlen. Cc: Jamal Hadi Salim Signed-off-by: Cong Wang --- net/sched/sch_dsmark.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/net/sched/sch_dsmark.c b/net/sched/sch_dsmark.c index cfddb1c..d0dff0c 100644 --- a/net/sched/sch_dsmark.c +++ b/net/sched/sch_dsmark.c @@ -258,6 +258,7 @@ static int dsmark_enqueue(struct sk_buff *skb, struct Qdisc *sch) return err; } + qdisc_qstats_backlog_inc(sch, skb); sch->q.qlen++; return NET_XMIT_SUCCESS; @@ -280,6 +281,7 @@ static struct sk_buff *dsmark_dequeue(struct Qdisc *sch) return NULL; qdisc_bstats_update(sch, skb); + qdisc_qstats_backlog_dec(sch, skb); sch->q.qlen--; index = skb->tc_index & (p->indices - 1); @@ -395,6 +397,7 @@ static void dsmark_reset(struct Qdisc *sch) pr_debug("%s(sch %p,[qdisc %p])\n", __func__, sch, p); qdisc_reset(p->q); + sch->qstats.backlog = 0; sch->q.qlen = 0; } -- 2.1.0
[Patch net-next v3 2/4] net_sched: update hierarchical backlog too
When the bottom qdisc decides to, for example, drop some packet, it calls qdisc_tree_decrease_qlen() to update the queue length for all its ancestors, we need to update the backlog too to keep the stats on root qdisc accurate. Cc: Jamal Hadi Salim Acked-by: Jamal Hadi Salim Signed-off-by: Cong Wang --- include/net/codel.h | 4 include/net/sch_generic.h | 5 +++-- net/sched/sch_api.c | 8 +--- net/sched/sch_cbq.c | 5 +++-- net/sched/sch_choke.c | 6 -- net/sched/sch_codel.c | 10 ++ net/sched/sch_drr.c | 3 ++- net/sched/sch_fq.c| 4 +++- net/sched/sch_fq_codel.c | 17 - net/sched/sch_hfsc.c | 3 ++- net/sched/sch_hhf.c | 10 +++--- net/sched/sch_htb.c | 10 ++ net/sched/sch_multiq.c| 8 +--- net/sched/sch_netem.c | 3 ++- net/sched/sch_pie.c | 5 +++-- net/sched/sch_prio.c | 7 --- net/sched/sch_qfq.c | 3 ++- net/sched/sch_red.c | 3 ++- net/sched/sch_sfb.c | 3 ++- net/sched/sch_sfq.c | 16 +--- net/sched/sch_tbf.c | 7 +-- 21 files changed, 91 insertions(+), 49 deletions(-) diff --git a/include/net/codel.h b/include/net/codel.h index 267e702..d168aca 100644 --- a/include/net/codel.h +++ b/include/net/codel.h @@ -162,12 +162,14 @@ struct codel_vars { * struct codel_stats - contains codel shared variables and stats * @maxpacket: largest packet we've seen so far * @drop_count:temp count of dropped packets in dequeue() + * @drop_len: bytes of dropped packets in dequeue() * ecn_mark: number of packets we ECN marked instead of dropping * ce_mark:number of packets CE marked because sojourn time was above ce_threshold */ struct codel_stats { u32 maxpacket; u32 drop_count; + u32 drop_len; u32 ecn_mark; u32 ce_mark; }; @@ -308,6 +310,7 @@ static struct sk_buff *codel_dequeue(struct Qdisc *sch, vars->rec_inv_sqrt); goto end; } + stats->drop_len += qdisc_pkt_len(skb); qdisc_drop(skb, sch); stats->drop_count++; skb = dequeue_func(vars, sch); @@ -330,6 +333,7 @@ static struct sk_buff *codel_dequeue(struct Qdisc *sch, if (params->ecn && INET_ECN_set_ce(skb)) { stats->ecn_mark++; } else { + stats->drop_len += qdisc_pkt_len(skb); qdisc_drop(skb, sch); stats->drop_count++; diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h index 8fdad9f..e5bba89 100644 --- a/include/net/sch_generic.h +++ b/include/net/sch_generic.h @@ -396,7 +396,8 @@ struct Qdisc *dev_graft_qdisc(struct netdev_queue *dev_queue, struct Qdisc *qdisc); void qdisc_reset(struct Qdisc *qdisc); void qdisc_destroy(struct Qdisc *qdisc); -void qdisc_tree_decrease_qlen(struct Qdisc *qdisc, unsigned int n); +void qdisc_tree_reduce_backlog(struct Qdisc *qdisc, unsigned int n, + unsigned int len); struct Qdisc *qdisc_alloc(struct netdev_queue *dev_queue, const struct Qdisc_ops *ops); struct Qdisc *qdisc_create_dflt(struct netdev_queue *dev_queue, @@ -716,7 +717,7 @@ static inline struct Qdisc *qdisc_replace(struct Qdisc *sch, struct Qdisc *new, old = *pold; *pold = new; if (old != NULL) { - qdisc_tree_decrease_qlen(old, old->q.qlen); + qdisc_tree_reduce_backlog(old, old->q.qlen, old->qstats.backlog); qdisc_reset(old); } sch_tree_unlock(sch); diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c index de1e176..3b180ff 100644 --- a/net/sched/sch_api.c +++ b/net/sched/sch_api.c @@ -744,14 +744,15 @@ static u32 qdisc_alloc_handle(struct net_device *dev) return 0; } -void qdisc_tree_decrease_qlen(struct Qdisc *sch, unsigned int n) +void qdisc_tree_reduce_backlog(struct Qdisc *sch, unsigned int n, + unsigned int len) { const struct Qdisc_class_ops *cops; unsigned long cl; u32 parentid; int drops; - if (n == 0) + if (n == 0 && len == 0) return; drops = max_t(int, n, 0); rcu_read_lock(); @@ -774,11 +775,12 @@ void qdisc_tree_decrease_qlen(struct Qdisc *sch, unsigned int n) cops->put(sch, cl); } sch->q.qlen -= n; + sch->qstats.backlog -= len; __qdisc_qstats_drop(sch, drops); } rcu_read_unlock(); } -EXPORT_SYMBOL(qdisc_tree_decrease_qlen); +EXPORT_SYMBOL(qdis
[Patch net-next v3 1/4] net_sched: introduce qdisc_replace() helper
Remove nearly duplicated code and prepare for the following patch. Cc: Jamal Hadi Salim Acked-by: Jamal Hadi Salim Signed-off-by: Cong Wang --- include/net/sch_generic.h | 17 + net/sched/sch_cbq.c | 7 +-- net/sched/sch_drr.c | 6 +- net/sched/sch_dsmark.c| 8 +--- net/sched/sch_hfsc.c | 6 +- net/sched/sch_htb.c | 9 + net/sched/sch_multiq.c| 8 +--- net/sched/sch_netem.c | 10 +- net/sched/sch_prio.c | 8 +--- net/sched/sch_qfq.c | 6 +- net/sched/sch_red.c | 7 +-- net/sched/sch_sfb.c | 7 +-- net/sched/sch_tbf.c | 8 +--- 13 files changed, 29 insertions(+), 78 deletions(-) diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h index 636a362..8fdad9f 100644 --- a/include/net/sch_generic.h +++ b/include/net/sch_generic.h @@ -707,6 +707,23 @@ static inline void qdisc_reset_queue(struct Qdisc *sch) sch->qstats.backlog = 0; } +static inline struct Qdisc *qdisc_replace(struct Qdisc *sch, struct Qdisc *new, + struct Qdisc **pold) +{ + struct Qdisc *old; + + sch_tree_lock(sch); + old = *pold; + *pold = new; + if (old != NULL) { + qdisc_tree_decrease_qlen(old, old->q.qlen); + qdisc_reset(old); + } + sch_tree_unlock(sch); + + return old; +} + static inline unsigned int __qdisc_queue_drop(struct Qdisc *sch, struct sk_buff_head *list) { diff --git a/net/sched/sch_cbq.c b/net/sched/sch_cbq.c index c538d9e..7f8474c 100644 --- a/net/sched/sch_cbq.c +++ b/net/sched/sch_cbq.c @@ -1624,13 +1624,8 @@ static int cbq_graft(struct Qdisc *sch, unsigned long arg, struct Qdisc *new, new->reshape_fail = cbq_reshape_fail; #endif } - sch_tree_lock(sch); - *old = cl->q; - cl->q = new; - qdisc_tree_decrease_qlen(*old, (*old)->q.qlen); - qdisc_reset(*old); - sch_tree_unlock(sch); + *old = qdisc_replace(sch, new, &cl->q); return 0; } diff --git a/net/sched/sch_drr.c b/net/sched/sch_drr.c index a1cd778..b96c9a8 100644 --- a/net/sched/sch_drr.c +++ b/net/sched/sch_drr.c @@ -226,11 +226,7 @@ static int drr_graft_class(struct Qdisc *sch, unsigned long arg, new = &noop_qdisc; } - sch_tree_lock(sch); - drr_purge_queue(cl); - *old = cl->qdisc; - cl->qdisc = new; - sch_tree_unlock(sch); + *old = qdisc_replace(sch, new, &cl->qdisc); return 0; } diff --git a/net/sched/sch_dsmark.c b/net/sched/sch_dsmark.c index f357f34..cfddb1c 100644 --- a/net/sched/sch_dsmark.c +++ b/net/sched/sch_dsmark.c @@ -73,13 +73,7 @@ static int dsmark_graft(struct Qdisc *sch, unsigned long arg, new = &noop_qdisc; } - sch_tree_lock(sch); - *old = p->q; - p->q = new; - qdisc_tree_decrease_qlen(*old, (*old)->q.qlen); - qdisc_reset(*old); - sch_tree_unlock(sch); - + *old = qdisc_replace(sch, new, &p->q); return 0; } diff --git a/net/sched/sch_hfsc.c b/net/sched/sch_hfsc.c index b7ebe2c..089f3b6 100644 --- a/net/sched/sch_hfsc.c +++ b/net/sched/sch_hfsc.c @@ -1215,11 +1215,7 @@ hfsc_graft_class(struct Qdisc *sch, unsigned long arg, struct Qdisc *new, new = &noop_qdisc; } - sch_tree_lock(sch); - hfsc_purge_queue(sch, cl); - *old = cl->qdisc; - cl->qdisc = new; - sch_tree_unlock(sch); + *old = qdisc_replace(sch, new, &cl->qdisc); return 0; } diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c index 15ccd7f..0efbcf3 100644 --- a/net/sched/sch_htb.c +++ b/net/sched/sch_htb.c @@ -1163,14 +1163,7 @@ static int htb_graft(struct Qdisc *sch, unsigned long arg, struct Qdisc *new, cl->common.classid)) == NULL) return -ENOBUFS; - sch_tree_lock(sch); - *old = cl->un.leaf.q; - cl->un.leaf.q = new; - if (*old != NULL) { - qdisc_tree_decrease_qlen(*old, (*old)->q.qlen); - qdisc_reset(*old); - } - sch_tree_unlock(sch); + *old = qdisc_replace(sch, new, &cl->un.leaf.q); return 0; } diff --git a/net/sched/sch_multiq.c b/net/sched/sch_multiq.c index 4e904ca..a0103a1 100644 --- a/net/sched/sch_multiq.c +++ b/net/sched/sch_multiq.c @@ -303,13 +303,7 @@ static int multiq_graft(struct Qdisc *sch, unsigned long arg, struct Qdisc *new, if (new == NULL) new = &noop_qdisc; - sch_tree_lock(sch); - *old = q->queues[band]; - q->queues[band] = new; - qdisc_tree_decrease_qlen(*old, (*old)->q.qlen); - qdisc_reset(*old); - sch_tree_unlock(sch); - + *old = qdisc_replace(sch, new, &q->queues[band]); return 0; } diff --git a/n
Re: [patch net-next v2 0/9] Introduce devlink interface and first drivers to use it
Thu, Feb 25, 2016 at 10:53:30PM CET, han...@stressinduktion.org wrote: >On 25.02.2016 22:12, Jiri Pirko wrote: >>Thu, Feb 25, 2016 at 09:44:43PM CET, han...@stressinduktion.org wrote: >>>On 25.02.2016 21:12, David Miller wrote: From: Jiri Pirko Date: Tue, 23 Feb 2016 16:51:25 +0100 >There a is need for some userspace API that would allow to expose things >that are not directly related to any device class like net_device of >ib_device, but rather chip-wide/switch-ASIC-wide stuff. > >Use cases: >1) get/set of port type (Ethernet/InfiniBand) >2) setting up port splitters - split port into multiple ones and squash >again, >enables usage of splitter cable >3) setting up shared buffers - shared among multiple ports within >one chip (work in progress) >4) configuration of switch wide properties - resources division etc - This >will >allow to pass configuration that is unacceptable to be passed as >a module option. > >First patch of this set introduces a new generic Netlink based interface, >called "devlink". It is similar to nl80211 model and it is heavily >influenced by it, including the API definition. The devlink introduction >patch >implements use cases 1) and 2). Other 2 are in development atm and will >be addressed by follow-ups. > >It is very convenient for drivers to use devlink, as you can see in other >patches in this set. > >Counterpart for devlink is userspace tool for now called "dl". Command line >interface and outputs are derived from "ip" tool so it should be easy >for users to get used to it. I am very close to applying this series as-is. The clincher for me is that there is precendence in the nl80211 stuff, so obviously whatever userland infrastructure sits on top of that has found a way to deal whatever perceived shortcomings devlink has. >>> >>>Actually nl80211 phy interfaces aren't really managed by wpa_supplicant nor >>>NetworkManager, but they use net_device names to discover those later on. In >>>devlink we don't necessarily have netdev names, thus my only objection to >>>this series is to switch to stable topology identifiers. >> >>Hannes, as I mentioned earlier in one of my replies, you can choose to >>use devlink name or pci (or other) address for your commands. So you >>have your stable names even before udev takes care of renaming. It's up >>to you as a user what handle to use. > >I understood that part from the beginning. In my opinion we should simply >keep APIs simple and clean, reduced to the minimum and let user space build >upon this. > >If an entity in the kernel doesn't need a name and/or a naming database, I >would not bother implementing it. Not everyone is a kernel developer who uses >Linux and can infer the problems from dl --help. I bet most users will use >names at some point in time and they simply can't match even with the udev >quirks in place when setting up netbooting or configuration must happen from >the currently provided initramfs. This is my point. > >If names are fragile to use simply don't provide it if not necessary. > >devlink is an interface of its own and the names won't be referenced by other >subsystems like iptables -i or iproute. So why bother with names in >the kernel? > >On the rest with netlink etc. I agree already. I just want to eliminate >possible mistakes by users. > >Hope that makes my point more clear, Okay. You convinced me. Will send v3 removing in-kernel naming and indexing of devlinks and will only use bus type and bus name as a handle. Thanks.
[PATCH v18 4/9] bpf: Mark __bpf_prog_run() stack frame as non-standard
objtool reports the following false positive warnings: objtool: kernel/bpf/core.o: __bpf_prog_run()+0x5c: sibling call from callable instruction with changed frame pointer objtool: kernel/bpf/core.o: __bpf_prog_run()+0x60: function has unreachable instruction objtool: kernel/bpf/core.o: __bpf_prog_run()+0x64: function has unreachable instruction [...] It's confused by the following dynamic jump instruction in __bpf_prog_run():: jmp *(%r12,%rax,8) which corresponds to the following line in the C code: goto *jumptable[insn->code]; There's no way for objtool to deterministically find all possible branch targets for a dynamic jump, so it can't verify this code. In this case the jumps all stay within the function, and there's nothing unusual going on related to the stack, so we can whitelist the function. Signed-off-by: Josh Poimboeuf Acked-by: Daniel Borkmann Acked-by: Alexei Starovoitov Cc: netdev@vger.kernel.org --- kernel/bpf/core.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c index 972d9a8..be0abf6 100644 --- a/kernel/bpf/core.c +++ b/kernel/bpf/core.c @@ -27,6 +27,7 @@ #include #include #include +#include #include @@ -649,6 +650,7 @@ load_byte: WARN_RATELIMIT(1, "unknown opcode %02x\n", insn->code); return 0; } +STACK_FRAME_NON_STANDARD(__bpf_prog_run); /* jump table */ bool bpf_prog_array_compatible(struct bpf_array *array, const struct bpf_prog *fp) -- 2.4.3
Re: [net-next PATCH 3/4] net: sched: cls_u32 add bit to specify software only rules
On 16-02-25 04:56 PM, John Fastabend wrote: On 16-02-25 04:56 AM, Jamal Hadi Salim wrote: decoding that is not a problem. The ixgbe driver code already applied can decode that without much trouble. The thing I want to avoid is requiring my driver to do the inverse translation because although it is possible its entirely unnecessary. To do the above example without a software cache for example means I read out row 2048 of a hardware table then you get a bunch of bits. From those bits I consult what fields are being loaded into the table in the packet processing pipeline. I learn its the src_ip fields then I have the value/mask and can unwind it. Finally if I collapsed some hash tables onto this hardware table I have to do the inverse of my collapsing scheme. The ixgbe one is sort of simple just because I only have one table in hardware but with multiple tables its a bit more difficult. Finally I've unwound the thing and can print something back out of 'tc' but it seems easier to just cache the hardware rules somewhere. Maybe other driver/hardware will have a different opinion though depending on how much your firmware can store and how ambitious you are. Personally I don't see any need for the above code. I think if you can cache the rules and have a way to easily map to the hardware then this would work fine. cheers, jamal
Re: [net-next PATCH 3/4] net: sched: cls_u32 add bit to specify software only rules
On 16-02-25 03:05 PM, Jamal Hadi Salim wrote: > On 16-02-25 04:56 PM, John Fastabend wrote: >> On 16-02-25 04:56 AM, Jamal Hadi Salim wrote: > >> >> decoding that is not a problem. The ixgbe driver code already applied >> can decode that without much trouble. The thing I want to avoid is >> requiring my driver to do the inverse translation because although it >> is possible its entirely unnecessary. >> >> To do the above example without a software cache for example >> means I read out row 2048 of a hardware table then you get a bunch of >> bits. From those bits I consult what fields are being loaded into >> the table in the packet processing pipeline. I learn its the src_ip >> fields then I have the value/mask and can unwind it. Finally if I >> collapsed some hash tables onto this hardware table I have to do the >> inverse of my collapsing scheme. The ixgbe one is sort of simple just >> because I only have one table in hardware but with multiple tables >> its a bit more difficult. Finally I've unwound the thing and can >> print something back out of 'tc' but it seems easier to just cache >> the hardware rules somewhere. Maybe other driver/hardware will have >> a different opinion though depending on how much your firmware can >> store and how ambitious you are. Personally I don't see any need >> for the above code. >> > > I think if you can cache the rules and have a way to easily map to > the hardware then this would work fine. Yep that is the goal. I think the debate is if its acceptable to do it with an entirely new filter list ingress_hw Jiri's opinion is that it would be best to do it inline inside the classifier. At the moment I'm looking at the code to see if there is a clean way to do it. IMO using a ingress_hw classifier is a nice solution. In the meantime I just respun patches 1-3 with the feedback and will submit those while I work out patch 4. > > cheers, > jamal
[PATCH][V2] mt7601u: do not free dma_buf when ivp allocation fails
From: Colin Ian King If the allocation of ivp fails the error handling attempts to free an uninitialized dma_buf; this data structure just contains garbage on the stack, so the freeing will cause issues when the urb, buf and dma fields are free'd. Fix this by not free'ing the dma_buf if the ivp allocation fails. Signed-off-by: Colin Ian King --- drivers/net/wireless/mediatek/mt7601u/mcu.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/net/wireless/mediatek/mt7601u/mcu.c b/drivers/net/wireless/mediatek/mt7601u/mcu.c index fbb1986..70e4b5e 100644 --- a/drivers/net/wireless/mediatek/mt7601u/mcu.c +++ b/drivers/net/wireless/mediatek/mt7601u/mcu.c @@ -362,10 +362,10 @@ mt7601u_upload_firmware(struct mt7601u_dev *dev, const struct mt76_fw *fw) int i, ret; ivb = kmemdup(fw->ivb, sizeof(fw->ivb), GFP_KERNEL); - if (!ivb || mt7601u_usb_alloc_buf(dev, MCU_FW_URB_SIZE, &dma_buf)) { - ret = -ENOMEM; + if (!ivb) + return -ENOMEM; + if (mt7601u_usb_alloc_buf(dev, MCU_FW_URB_SIZE, &dma_buf)) goto error; - } ilm_len = le32_to_cpu(fw->hdr.ilm_len) - sizeof(fw->ivb); dev_dbg(dev->dev, "loading FW - ILM %u + IVB %zu\n", -- 2.7.0
Re: [PATCH][V2] mt7601u: do not free dma_buf when ivp allocation fails
Hi Colin, On Fri, Feb 26, 2016 at 10:09 AM, Colin King wrote: > From: Colin Ian King > > If the allocation of ivp fails the error handling attempts to > free an uninitialized dma_buf; this data structure just contains > garbage on the stack, so the freeing will cause issues when the > urb, buf and dma fields are free'd. Fix this by not free'ing the > dma_buf if the ivp allocation fails. > > Signed-off-by: Colin Ian King > --- > drivers/net/wireless/mediatek/mt7601u/mcu.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/wireless/mediatek/mt7601u/mcu.c > b/drivers/net/wireless/mediatek/mt7601u/mcu.c > index fbb1986..70e4b5e 100644 > --- a/drivers/net/wireless/mediatek/mt7601u/mcu.c > +++ b/drivers/net/wireless/mediatek/mt7601u/mcu.c > @@ -362,10 +362,10 @@ mt7601u_upload_firmware(struct mt7601u_dev *dev, const > struct mt76_fw *fw) > int i, ret; > > ivb = kmemdup(fw->ivb, sizeof(fw->ivb), GFP_KERNEL); > - if (!ivb || mt7601u_usb_alloc_buf(dev, MCU_FW_URB_SIZE, &dma_buf)) { > - ret = -ENOMEM; > + if (!ivb) > + return -ENOMEM; > + if (mt7601u_usb_alloc_buf(dev, MCU_FW_URB_SIZE, &dma_buf)) > goto error; Are you sure this is right? Isn't ret unset here and consequently returned at the end of the error label? Thanks, -- Julian Calaby Email: julian.cal...@gmail.com Profile: http://www.google.com/profiles/julian.calaby/
[net-next PATCH v2 0/3] tc software only flag
This adds a software only flag to tc but incorporates a bunch of comments from the original attempt at this. First instead of having the offload decision logic be embedded in cls_u32 I lifted into cls_pkt.h so it can be used anywhere. In order to do this I put the flag defines in pkt_cls.h as well. However it was suggested that perhaps these flags could be lifted into the upper layer of TCA_ as well but I'm afraid this can not be done with existing tc design as far as I can tell. The problem is the filters are packed and unpacked in the classifier specific code and pushing the flags through the high level doesn't seem easily doable. And we already have this design where classifiers handle generic options such as actions and policers. So I think adding one more thing here is OK as 'tc', et. al. already know how to handle this type of thing. Thanks, .John --- John Fastabend (3): net: sched: consolidate offload decision in cls_u32 net: cls_u32: move TC offload feature bit into cls_u32 offload logic net: sched: cls_u32 add bit to specify software only rules drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |3 -- include/net/pkt_cls.h | 17 +++ include/uapi/linux/pkt_cls.h |1 + net/sched/cls_u32.c | 37 ++--- 4 files changed, 45 insertions(+), 13 deletions(-) -- Signature
[net-next PATCH v2 1/3] net: sched: consolidate offload decision in cls_u32
The offload decision was originally very basic and tied to if the dev implemented the appropriate ndo op hook. The next step is to allow the user to more flexibly define if any paticular rule should be offloaded or not. In order to have this logic in one function lift the current check into a helper routine tc_should_offload(). Signed-off-by: John Fastabend --- include/net/pkt_cls.h |5 + net/sched/cls_u32.c |8 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h index 2121df5..e64d20b 100644 --- a/include/net/pkt_cls.h +++ b/include/net/pkt_cls.h @@ -392,4 +392,9 @@ struct tc_cls_u32_offload { }; }; +static inline bool tc_should_offload(struct net_device *dev) +{ + return dev->netdev_ops->ndo_setup_tc; +} + #endif diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c index d54bc94..24e888b 100644 --- a/net/sched/cls_u32.c +++ b/net/sched/cls_u32.c @@ -434,7 +434,7 @@ static void u32_remove_hw_knode(struct tcf_proto *tp, u32 handle) offload.type = TC_SETUP_CLSU32; offload.cls_u32 = &u32_offload; - if (dev->netdev_ops->ndo_setup_tc) { + if (tc_should_offload(dev)) { offload.cls_u32->command = TC_CLSU32_DELETE_KNODE; offload.cls_u32->knode.handle = handle; dev->netdev_ops->ndo_setup_tc(dev, tp->q->handle, @@ -451,7 +451,7 @@ static void u32_replace_hw_hnode(struct tcf_proto *tp, struct tc_u_hnode *h) offload.type = TC_SETUP_CLSU32; offload.cls_u32 = &u32_offload; - if (dev->netdev_ops->ndo_setup_tc) { + if (tc_should_offload(dev)) { offload.cls_u32->command = TC_CLSU32_NEW_HNODE; offload.cls_u32->hnode.divisor = h->divisor; offload.cls_u32->hnode.handle = h->handle; @@ -471,7 +471,7 @@ static void u32_clear_hw_hnode(struct tcf_proto *tp, struct tc_u_hnode *h) offload.type = TC_SETUP_CLSU32; offload.cls_u32 = &u32_offload; - if (dev->netdev_ops->ndo_setup_tc) { + if (tc_should_offload(dev)) { offload.cls_u32->command = TC_CLSU32_DELETE_HNODE; offload.cls_u32->hnode.divisor = h->divisor; offload.cls_u32->hnode.handle = h->handle; @@ -491,7 +491,7 @@ static void u32_replace_hw_knode(struct tcf_proto *tp, struct tc_u_knode *n) offload.type = TC_SETUP_CLSU32; offload.cls_u32 = &u32_offload; - if (dev->netdev_ops->ndo_setup_tc) { + if (tc_should_offload(dev)) { offload.cls_u32->command = TC_CLSU32_REPLACE_KNODE; offload.cls_u32->knode.handle = n->handle; offload.cls_u32->knode.fshift = n->fshift;
[net-next PATCH v2 2/3] net: cls_u32: move TC offload feature bit into cls_u32 offload logic
In the original series drivers would get offload requests for cls_u32 rules even if the feature bit is disabled. This meant the driver had to do a boiler plate check on the feature bit before adding/deleting the rule. This patch lifts the check into the core code and removes it from the driver specific case. Signed-off-by: John Fastabend --- drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |3 --- include/net/pkt_cls.h |3 +++ 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c index cf4b729..b893ff8 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c @@ -8400,9 +8400,6 @@ int __ixgbe_setup_tc(struct net_device *dev, u32 handle, __be16 proto, if (TC_H_MAJ(handle) == TC_H_MAJ(TC_H_INGRESS) && tc->type == TC_SETUP_CLSU32) { - if (!(dev->features & NETIF_F_HW_TC)) - return -EINVAL; - switch (tc->cls_u32->command) { case TC_CLSU32_NEW_KNODE: case TC_CLSU32_REPLACE_KNODE: diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h index e64d20b..6096e96 100644 --- a/include/net/pkt_cls.h +++ b/include/net/pkt_cls.h @@ -394,6 +394,9 @@ struct tc_cls_u32_offload { static inline bool tc_should_offload(struct net_device *dev) { + if (!(dev->features & NETIF_F_HW_TC)) + return false; + return dev->netdev_ops->ndo_setup_tc; }
[net-next PATCH v2 3/3] net: sched: cls_u32 add bit to specify software only rules
In the initial implementation the only way to stop a rule from being inserted into the hardware table was via the device feature flag. However this doesn't work well when working on an end host system where packets are expect to hit both the hardware and software datapaths. For example we can imagine a rule that will match an IP address and increment a field. If we install this rule in both hardware and software we may increment the field twice. To date we have only added support for the drop action so we have been able to ignore these cases. But as we extend the action support we will hit this example plus more such cases. Arguably these are not even corner cases in many working systems these cases will be common. To avoid forcing the driver to always abort (i.e. the above example) this patch adds a flag to add a rule in software only. A careful user can use this flag to build software and hardware datapaths that work together. One example we have found particularly useful is to use hardware resources to set the skb->mark on the skb when the match may be expensive to run in software but a mark lookup in a hash table is cheap. The idea here is hardware can do in one lookup what the u32 classifier may need to traverse multiple lists and hash tables to compute. The flag is only passed down on inserts on deletion to avoid stale references in hardware we always try to remove a rule if it exists. The flags field is part of the classifier specific options. Although it is tempting to lift this into the generic structure doing this proves difficult do to how the tc netlink attributes are implemented along with how the dump/change routines are called. There is also precedence for putting seemingly generic pieces in the specific classifier options such as TCA_U32_POLICE, TCA_U32_ACT, etc. So although not ideal I've left FLAGS in the u32 options as well as it simplifies the code greatly and user space has already learned how to manage these bits ala 'tc' tool. Another thing if trying to update a rule we require the flags to be unchanged. This is to force user space, software u32 and the hardware u32 to keep in sync. Thanks to Simon Horman for catching this case. Signed-off-by: John Fastabend --- include/net/pkt_cls.h| 13 +++-- include/uapi/linux/pkt_cls.h |1 + net/sched/cls_u32.c | 37 +++-- 3 files changed, 39 insertions(+), 12 deletions(-) diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h index 6096e96..42dc412 100644 --- a/include/net/pkt_cls.h +++ b/include/net/pkt_cls.h @@ -392,12 +392,21 @@ struct tc_cls_u32_offload { }; }; -static inline bool tc_should_offload(struct net_device *dev) +/* tca flags definitions */ +#define TCA_CLS_FLAGS_SOFTWARE 1 + +static inline bool tc_should_offload(struct net_device *dev, u32 flags) { if (!(dev->features & NETIF_F_HW_TC)) return false; - return dev->netdev_ops->ndo_setup_tc; + if (flags & TCA_CLS_FLAGS_SOFTWARE) + return false; + + if (!dev->netdev_ops->ndo_setup_tc) + return false; + + return true; } #endif diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h index 4398737..9874f568 100644 --- a/include/uapi/linux/pkt_cls.h +++ b/include/uapi/linux/pkt_cls.h @@ -172,6 +172,7 @@ enum { TCA_U32_INDEV, TCA_U32_PCNT, TCA_U32_MARK, + TCA_U32_FLAGS, __TCA_U32_MAX }; diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c index 24e888b..6d4450d 100644 --- a/net/sched/cls_u32.c +++ b/net/sched/cls_u32.c @@ -59,6 +59,7 @@ struct tc_u_knode { #ifdef CONFIG_CLS_U32_PERF struct tc_u32_pcnt __percpu *pf; #endif + u32 flags; #ifdef CONFIG_CLS_U32_MARK u32 val; u32 mask; @@ -434,7 +435,7 @@ static void u32_remove_hw_knode(struct tcf_proto *tp, u32 handle) offload.type = TC_SETUP_CLSU32; offload.cls_u32 = &u32_offload; - if (tc_should_offload(dev)) { + if (tc_should_offload(dev, 0)) { offload.cls_u32->command = TC_CLSU32_DELETE_KNODE; offload.cls_u32->knode.handle = handle; dev->netdev_ops->ndo_setup_tc(dev, tp->q->handle, @@ -442,7 +443,9 @@ static void u32_remove_hw_knode(struct tcf_proto *tp, u32 handle) } } -static void u32_replace_hw_hnode(struct tcf_proto *tp, struct tc_u_hnode *h) +static void u32_replace_hw_hnode(struct tcf_proto *tp, +struct tc_u_hnode *h, +u32 flags) { struct net_device *dev = tp->q->dev_queue->dev; struct tc_cls_u32_offload u32_offload = {0}; @@ -451,7 +454,7 @@ static void u32_replace_hw_hnode(struct tcf_proto *tp, struct tc_u_hnode *h) offload.type = TC_SETUP_CLSU32; offload.cls_u32 = &u32_offload; - if (tc_should_offload(dev)) { + if
Re: [net-next PATCH v2 0/3] tc software only flag
On 16-02-25 03:19 PM, John Fastabend wrote: > This adds a software only flag to tc but incorporates a bunch of comments > from the original attempt at this. In case its not entirely obvious I dropped the hardware only case for now because I'm investigating Jiri's comment. But just having the software only case lets me handle many use cases. Now in the u32 classifier I can build graphs where part of the graph is software only and other parts are sw/hw. This lets me use part of the mark bit or queue information to learn if the hardware has already acted on the skb and then parse it differently in the classifier. > > First instead of having the offload decision logic be embedded in cls_u32 > I lifted into cls_pkt.h so it can be used anywhere. > > In order to do this I put the flag defines in pkt_cls.h as well. However > it was suggested that perhaps these flags could be lifted into the > upper layer of TCA_ as well but I'm afraid this can not be done with > existing tc design as far as I can tell. The problem is the filters are > packed and unpacked in the classifier specific code and pushing the flags > through the high level doesn't seem easily doable. And we already have > this design where classifiers handle generic options such as actions and > policers. So I think adding one more thing here is OK as 'tc', et. al. > already know how to handle this type of thing. > > Thanks, > .John > > --- > > John Fastabend (3): > net: sched: consolidate offload decision in cls_u32 > net: cls_u32: move TC offload feature bit into cls_u32 offload logic > net: sched: cls_u32 add bit to specify software only rules > > > drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |3 -- > include/net/pkt_cls.h | 17 +++ > include/uapi/linux/pkt_cls.h |1 + > net/sched/cls_u32.c | 37 > ++--- > 4 files changed, 45 insertions(+), 13 deletions(-) > > -- > Signature >
[PATCH][V3] mt7601u: do not free dma_buf when ivp allocation fails
From: Colin Ian King If the allocation of ivp fails the error handling attempts to free an uninitialized dma_buf; this data structure just contains garbage on the stack, so the freeing will cause issues when the urb, buf and dma fields are free'd. Fix this by not free'ing the dma_buf if the ivp allocation fails. Signed-off-by: Colin Ian King --- drivers/net/wireless/mediatek/mt7601u/mcu.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/net/wireless/mediatek/mt7601u/mcu.c b/drivers/net/wireless/mediatek/mt7601u/mcu.c index fbb1986..91c4b34 100644 --- a/drivers/net/wireless/mediatek/mt7601u/mcu.c +++ b/drivers/net/wireless/mediatek/mt7601u/mcu.c @@ -362,7 +362,9 @@ mt7601u_upload_firmware(struct mt7601u_dev *dev, const struct mt76_fw *fw) int i, ret; ivb = kmemdup(fw->ivb, sizeof(fw->ivb), GFP_KERNEL); - if (!ivb || mt7601u_usb_alloc_buf(dev, MCU_FW_URB_SIZE, &dma_buf)) { + if (!ivb) + return -ENOMEM; + if (mt7601u_usb_alloc_buf(dev, MCU_FW_URB_SIZE, &dma_buf)) { ret = -ENOMEM; goto error; } -- 2.7.0
Re: [PATCH][V3] mt7601u: do not free dma_buf when ivp allocation fails
Hi, On Fri, Feb 26, 2016 at 10:24 AM, Colin King wrote: > From: Colin Ian King > > If the allocation of ivp fails the error handling attempts to > free an uninitialized dma_buf; this data structure just contains > garbage on the stack, so the freeing will cause issues when the > urb, buf and dma fields are free'd. Fix this by not free'ing the > dma_buf if the ivp allocation fails. > > Signed-off-by: Colin Ian King Looks right to me. Reviewed-by: Julian Calaby > --- > drivers/net/wireless/mediatek/mt7601u/mcu.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/wireless/mediatek/mt7601u/mcu.c > b/drivers/net/wireless/mediatek/mt7601u/mcu.c > index fbb1986..91c4b34 100644 > --- a/drivers/net/wireless/mediatek/mt7601u/mcu.c > +++ b/drivers/net/wireless/mediatek/mt7601u/mcu.c > @@ -362,7 +362,9 @@ mt7601u_upload_firmware(struct mt7601u_dev *dev, const > struct mt76_fw *fw) > int i, ret; > > ivb = kmemdup(fw->ivb, sizeof(fw->ivb), GFP_KERNEL); > - if (!ivb || mt7601u_usb_alloc_buf(dev, MCU_FW_URB_SIZE, &dma_buf)) { > + if (!ivb) > + return -ENOMEM; > + if (mt7601u_usb_alloc_buf(dev, MCU_FW_URB_SIZE, &dma_buf)) { > ret = -ENOMEM; > goto error; > } > -- > 2.7.0 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-wireless" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html Thanks, -- Julian Calaby Email: julian.cal...@gmail.com Profile: http://www.google.com/profiles/julian.calaby/
[PATCH net-next 0/4] lan78xx: driver update
This patch series add new ethtool functions of set_pauseparam & get_pauseparam and MAINTAINERS entry. Woojung Huh (4): lan78xx: replace devid to chipid & chiprev lan78xx: remove unnecessary code lan78xx: add ethtool set & get pause functions MAINTAINERS: Add LAN78XX entry MAINTAINERS | 7 +++ drivers/net/usb/lan78xx.c | 107 ++ drivers/net/usb/lan78xx.h | 1 + 3 files changed, 97 insertions(+), 18 deletions(-) -- 2.7.0
[PATCH net-next 3/4] lan78xx: add ethtool set & get pause functions
Add ethtool operations of set_pauseram and get_pauseparm. Signed-off-by: Woojung Huh --- drivers/net/usb/lan78xx.c | 80 +-- 1 file changed, 77 insertions(+), 3 deletions(-) diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c index d100421..705c180 100644 --- a/drivers/net/usb/lan78xx.c +++ b/drivers/net/usb/lan78xx.c @@ -36,7 +36,7 @@ #define DRIVER_AUTHOR "WOOJUNG HUH " #define DRIVER_DESC"LAN78XX USB 3.0 Gigabit Ethernet Devices" #define DRIVER_NAME"lan78xx" -#define DRIVER_VERSION "1.0.2" +#define DRIVER_VERSION "1.0.3" #define TX_TIMEOUT_JIFFIES (5 * HZ) #define THROTTLE_JIFFIES (HZ / 8) @@ -281,6 +281,9 @@ struct lan78xx_net { u32 chipid; u32 chiprev; struct mii_bus *mdiobus; + + int fc_autoneg; + u8 fc_request_control; }; /* use ethtool to change the level for any given device */ @@ -902,11 +905,15 @@ static int lan78xx_update_flowcontrol(struct lan78xx_net *dev, u8 duplex, { u32 flow = 0, fct_flow = 0; int ret; + u8 cap; - u8 cap = mii_resolve_flowctrl_fdx(lcladv, rmtadv); + if (dev->fc_autoneg) + cap = mii_resolve_flowctrl_fdx(lcladv, rmtadv); + else + cap = dev->fc_request_control; if (cap & FLOW_CTRL_TX) - flow = (FLOW_CR_TX_FCEN_ | 0x); + flow |= (FLOW_CR_TX_FCEN_ | 0x); if (cap & FLOW_CTRL_RX) flow |= FLOW_CR_RX_FCEN_; @@ -1386,6 +1393,62 @@ static int lan78xx_set_settings(struct net_device *net, struct ethtool_cmd *cmd) return ret; } +static void lan78xx_get_pause(struct net_device *net, + struct ethtool_pauseparam *pause) +{ + struct lan78xx_net *dev = netdev_priv(net); + struct phy_device *phydev = net->phydev; + struct ethtool_cmd ecmd = { .cmd = ETHTOOL_GSET }; + + phy_ethtool_gset(phydev, &ecmd); + + pause->autoneg = dev->fc_autoneg; + + if (dev->fc_request_control & FLOW_CTRL_TX) + pause->tx_pause = 1; + + if (dev->fc_request_control & FLOW_CTRL_RX) + pause->rx_pause = 1; +} + +static int lan78xx_set_pause(struct net_device *net, +struct ethtool_pauseparam *pause) +{ + struct lan78xx_net *dev = netdev_priv(net); + struct phy_device *phydev = net->phydev; + struct ethtool_cmd ecmd = { .cmd = ETHTOOL_GSET }; + int ret; + + phy_ethtool_gset(phydev, &ecmd); + + if (pause->autoneg && !ecmd.autoneg) { + ret = -EINVAL; + goto exit; + } + + dev->fc_request_control = 0; + if (pause->rx_pause) + dev->fc_request_control |= FLOW_CTRL_RX; + + if (pause->tx_pause) + dev->fc_request_control |= FLOW_CTRL_TX; + + if (ecmd.autoneg) { + u32 mii_adv; + + ecmd.advertising &= ~(ADVERTISED_Pause | ADVERTISED_Asym_Pause); + mii_adv = (u32)mii_advertise_flowctrl(dev->fc_request_control); + ecmd.advertising |= mii_adv_to_ethtool_adv_t(mii_adv); + phy_ethtool_sset(phydev, &ecmd); + } + + dev->fc_autoneg = pause->autoneg; + + ret = 0; +exit: + return ret; +} + static const struct ethtool_ops lan78xx_ethtool_ops = { .get_link = lan78xx_get_link, .nway_reset = lan78xx_nway_reset, @@ -1404,6 +1467,8 @@ static const struct ethtool_ops lan78xx_ethtool_ops = { .set_wol= lan78xx_set_wol, .get_eee= lan78xx_get_eee, .set_eee= lan78xx_set_eee, + .get_pauseparam = lan78xx_get_pause, + .set_pauseparam = lan78xx_set_pause, }; static int lan78xx_ioctl(struct net_device *netdev, struct ifreq *rq, int cmd) @@ -1591,6 +1656,7 @@ static void lan78xx_link_status_change(struct net_device *net) static int lan78xx_phy_init(struct lan78xx_net *dev) { int ret; + u32 mii_adv; struct phy_device *phydev = dev->net->phydev; phydev = phy_find_first(dev->mdiobus); @@ -1624,8 +1690,16 @@ static int lan78xx_phy_init(struct lan78xx_net *dev) /* MAC doesn't support 1000T Half */ phydev->supported &= ~SUPPORTED_1000baseT_Half; + /* support both flow controls */ + dev->fc_request_control = (FLOW_CTRL_RX | FLOW_CTRL_TX); + phydev->advertising &= ~(ADVERTISED_Pause | ADVERTISED_Asym_Pause); + mii_adv = (u32)mii_advertise_flowctrl(dev->fc_request_control); + phydev->advertising |= mii_adv_to_ethtool_adv_t(mii_adv); + genphy_config_aneg(phydev); + dev->fc_autoneg = phydev->autoneg; + phy_start(phydev); netif_dbg(dev, ifup, dev->net, "phy initialised successfully"); -- 2.7.0
[PATCH net-next 1/4] lan78xx: replace devid to chipid & chiprev
Replace devid to chipid & chiprev for easy access. Signed-off-by: Woojung Huh --- drivers/net/usb/lan78xx.c | 20 +++- drivers/net/usb/lan78xx.h | 1 + 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c index 1c299b8..4ec25e8 100644 --- a/drivers/net/usb/lan78xx.c +++ b/drivers/net/usb/lan78xx.c @@ -278,7 +278,8 @@ struct lan78xx_net { int link_on; u8 mdix_ctrl; - u32 devid; + u32 chipid; + u32 chiprev; struct mii_bus *mdiobus; }; @@ -471,7 +472,7 @@ static int lan78xx_read_raw_eeprom(struct lan78xx_net *dev, u32 offset, */ ret = lan78xx_read_reg(dev, HW_CFG, &val); saved = val; - if ((dev->devid & ID_REV_CHIP_ID_MASK_) == 0x7800) { + if (dev->chipid == ID_REV_CHIP_ID_7800_) { val &= ~(HW_CFG_LED1_EN_ | HW_CFG_LED0_EN_); ret = lan78xx_write_reg(dev, HW_CFG, val); } @@ -505,7 +506,7 @@ static int lan78xx_read_raw_eeprom(struct lan78xx_net *dev, u32 offset, retval = 0; exit: - if ((dev->devid & ID_REV_CHIP_ID_MASK_) == 0x7800) + if (dev->chipid == ID_REV_CHIP_ID_7800_) ret = lan78xx_write_reg(dev, HW_CFG, saved); return retval; @@ -539,7 +540,7 @@ static int lan78xx_write_raw_eeprom(struct lan78xx_net *dev, u32 offset, */ ret = lan78xx_read_reg(dev, HW_CFG, &val); saved = val; - if ((dev->devid & ID_REV_CHIP_ID_MASK_) == 0x7800) { + if (dev->chipid == ID_REV_CHIP_ID_7800_) { val &= ~(HW_CFG_LED1_EN_ | HW_CFG_LED0_EN_); ret = lan78xx_write_reg(dev, HW_CFG, val); } @@ -587,7 +588,7 @@ static int lan78xx_write_raw_eeprom(struct lan78xx_net *dev, u32 offset, retval = 0; exit: - if ((dev->devid & ID_REV_CHIP_ID_MASK_) == 0x7800) + if (dev->chipid == ID_REV_CHIP_ID_7800_) ret = lan78xx_write_reg(dev, HW_CFG, saved); return retval; @@ -1555,9 +1556,9 @@ static int lan78xx_mdio_init(struct lan78xx_net *dev) snprintf(dev->mdiobus->id, MII_BUS_ID_SIZE, "usb-%03d:%03d", dev->udev->bus->busnum, dev->udev->devnum); - switch (dev->devid & ID_REV_CHIP_ID_MASK_) { - case 0x7800: - case 0x7850: + switch (dev->chipid) { + case ID_REV_CHIP_ID_7800_: + case ID_REV_CHIP_ID_7850_: /* set to internal PHY id */ dev->mdiobus->phy_mask = ~(1 << 1); break; @@ -1918,7 +1919,8 @@ static int lan78xx_reset(struct lan78xx_net *dev) /* save DEVID for later usage */ ret = lan78xx_read_reg(dev, ID_REV, &buf); - dev->devid = buf; + dev->chipid = (buf & ID_REV_CHIP_ID_MASK_) >> 16; + dev->chiprev = buf & ID_REV_CHIP_REV_MASK_; /* Respond to the IN token with a NAK */ ret = lan78xx_read_reg(dev, USB_CFG0, &buf); diff --git a/drivers/net/usb/lan78xx.h b/drivers/net/usb/lan78xx.h index a93fb65..4092790 100644 --- a/drivers/net/usb/lan78xx.h +++ b/drivers/net/usb/lan78xx.h @@ -107,6 +107,7 @@ #define ID_REV_CHIP_ID_MASK_ (0x) #define ID_REV_CHIP_REV_MASK_ (0x) #define ID_REV_CHIP_ID_7800_ (0x7800) +#define ID_REV_CHIP_ID_7850_ (0x7850) #define FPGA_REV (0x04) #define FPGA_REV_MINOR_MASK_ (0xFF00) -- 2.7.0
[PATCH net-next 4/4] MAINTAINERS: Add LAN78XX entry
Add maintainers for Microchip LAN78XX. unglinuxdri...@microchip.com is alias email which goes to current developers work for Microchip Network related products. Signed-off-by: Woojung Huh --- MAINTAINERS | 7 +++ 1 file changed, 7 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 27393cf..12b764f 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -11332,6 +11332,13 @@ S: Maintained F: drivers/usb/host/isp116x* F: include/linux/usb/isp116x.h +USB LAN78XX ETHERNET DRIVER +M: Woojung Huh +M: Microchip Linux Driver Support +L: netdev@vger.kernel.org +S: Maintained +F: drivers/net/usb/lan78xx.* + USB MASS STORAGE DRIVER M: Matthew Dharm L: linux-...@vger.kernel.org -- 2.7.0
[PATCH net-next 2/4] lan78xx: remove unnecessary code
It is not required after commit cd772de358d6 ("phy: keep pause flags in phy driver features") Signed-off-by: Woojung Huh --- drivers/net/usb/lan78xx.c | 7 +-- 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c index 4ec25e8..d100421 100644 --- a/drivers/net/usb/lan78xx.c +++ b/drivers/net/usb/lan78xx.c @@ -1623,12 +1623,7 @@ static int lan78xx_phy_init(struct lan78xx_net *dev) /* MAC doesn't support 1000T Half */ phydev->supported &= ~SUPPORTED_1000baseT_Half; - phydev->supported |= (SUPPORTED_10baseT_Half | - SUPPORTED_10baseT_Full | - SUPPORTED_100baseT_Half | - SUPPORTED_100baseT_Full | - SUPPORTED_1000baseT_Full | - SUPPORTED_Pause | SUPPORTED_Asym_Pause); + genphy_config_aneg(phydev); phy_start(phydev); -- 2.7.0
Re: [PATCH][V3] mt7601u: do not free dma_buf when ivp allocation fails
On 25 February 2016 18:24:27 GMT-05:00, Colin King wrote: >From: Colin Ian King > >If the allocation of ivp fails the error handling attempts to >free an uninitialized dma_buf; this data structure just contains >garbage on the stack, so the freeing will cause issues when the >urb, buf and dma fields are free'd. Fix this by not free'ing the >dma_buf if the ivp allocation fails. > >Signed-off-by: Colin Ian King LGTM, thanks.
Re: [net-next PATCH 0/5] net_sched: Add support for IFE action
On 02/25/2016 11:40 PM, Jamal Hadi Salim wrote: On 16-02-25 04:34 PM, Daniel Borkmann wrote: On 02/25/2016 01:23 PM, Jamal Hadi Salim wrote: On 16-02-24 12:48 PM, Daniel Borkmann wrote: On 02/24/2016 01:49 PM, Jamal Hadi Salim wrote: [...] Drivers do set the hash. My use case is slightly different. I have a NIC which has an embedded cavium processor. This thing strips off the TLV and uses the hash to select the host MSI. Only thing we dont use at the moment is queue_mapping. Ok, but the example says ingress qdisc. ;) I presume the driver for the NIC and the offloading parts are non-public? :/ Well, it is not exactly mellanox or intel - but I am sure someone would be willing to sell that NIC. So, without them, placing this on ingress qdisc doesn't seem much useful wrt the skb hash example, and most people only have the software part (for ingress I mean) available. Do you want me to take skbbhash out? I just want to get this set in then worry about adding and modifying. Well, if the NIC driver and offloading parts for ife are not available (or cannot be made available) If you are willing to pay for one i can have one sold to you. Note: There is no dependency on such a NIC. You asked for an example of how one would use skbhash and i pointed it out to you. Infact nothing in the commit notes pointed to any NIC. Yes, but that was not the point, I think we're talking past each other. ;) The example (or better, use-case) you provided with your NIC seems useful. But when you want to decode this with your kernel module, f.e. attached on ingress qdisc, it's an entirely different situation. Anyway, point is subsequent calls to skb_get_hash() will overwrite what you decoded there, that's why I asked and tried to point this out earlier. You may want to look at sth like __skb_set_sw_hash() ... in the upstream kernel tree, then you have to assume that everyone else can _only_ use this with the existing tc facilities in the kernel. And as such, the whole set needs to be evaluated, I think this is nothing new. ;-) Seriously this is getting to a ridiculous level now. I gave a talk - which you attended. I wrote a paper which you may have at minimal glanced at. I have had discussions with you on this very subject. I think I only suggested that in case you still don't have an ethertype assigned to this to require the user to specify one instead of some default. And as soon as i posted the patches your statements led from you needing a good use case to why not use a netdev and why i have all these things that are not very useful. None of which came up before. Fair enough, didn't see that code before and just tried to review it, probably shouldn't have, so I'm not looking further into this anymore. ;) Thanks anyway, Daniel
Re: [PATCH net] ipv4: only create late gso-skb if skb is already set up with CHECKSUM_PARTIAL
Hannes Frederic Sowa wrote: > Otherwise we break the contract with GSO to only pass CHECKSUM_PARTIAL > skbs down. This can easily happen with UDP+IPv4 sockets with the first > MSG_MORE write smaller than the MTU, second write is a sendfile. > > Returning -EOPNOTSUPP lets the callers fall back into normal sendmsg path, > were we calculate the checksum manually during copying. > > Commit d749c9cbffd6 ("ipv4: no CHECKSUM_PARTIAL on MSG_MORE corked > sockets") started to exposes this bug. > > Fixes: d749c9cbffd6 ("ipv4: no CHECKSUM_PARTIAL on MSG_MORE corked sockets") > Reported-by: Jiri Benc > Cc: Jiri Benc > Reported-by: Wakko Warner > Cc: Wakko Warner > Signed-off-by: Hannes Frederic Sowa I just tested this with 2 of my VMs. It appears to have fixed the issue. > --- > net/ipv4/ip_output.c | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c > index 64878efa045c13..565bf64b2b7d60 100644 > --- a/net/ipv4/ip_output.c > +++ b/net/ipv4/ip_output.c > @@ -1236,13 +1236,16 @@ ssize_t ip_append_page(struct sock *sk, struct > flowi4 *fl4, struct page *page, > if (!skb) > return -EINVAL; > > - cork->length += size; > if ((size + skb->len > mtu) && > (sk->sk_protocol == IPPROTO_UDP) && > (rt->dst.dev->features & NETIF_F_UFO)) { > + if (skb->ip_summed != CHECKSUM_PARTIAL) > + return -EOPNOTSUPP; > + > skb_shinfo(skb)->gso_size = mtu - fragheaderlen; > skb_shinfo(skb)->gso_type = SKB_GSO_UDP; > } > + cork->length += size; > > while (size > 0) { > if (skb_is_gso(skb)) { > -- > 2.5.0 > -- Microsoft has beaten Volkswagen's world record. Volkswagen only created 22 million bugs.
linux-next: manual merge of the net-next tree with the net tree
Hi all, Today's linux-next merge of the net-next tree got a conflict in: include/uapi/linux/bpf.h between commit: 2da897e51d7f ("bpf: fix csum setting for bpf_set_tunnel_key") from the net tree and commit: d5a3b1f69186 ("bpf: introduce BPF_MAP_TYPE_STACK_TRACE") from the net-next tree. I fixed it up (see below) and can carry the fix as necessary (no action is required). -- Cheers, Stephen Rothwell diff --cc include/uapi/linux/bpf.h index 5df4881dea7b,6496f98d3d68.. --- a/include/uapi/linux/bpf.h +++ b/include/uapi/linux/bpf.h @@@ -292,9 -321,12 +321,15 @@@ enum bpf_func_id /* BPF_FUNC_skb_set_tunnel_key and BPF_FUNC_skb_get_tunnel_key flags. */ #define BPF_F_TUNINFO_IPV6(1ULL << 0) +/* BPF_FUNC_skb_set_tunnel_key flags. */ +#define BPF_F_ZERO_CSUM_TX(1ULL << 1) + + /* BPF_FUNC_get_stackid flags. */ + #define BPF_F_SKIP_FIELD_MASK 0xffULL + #define BPF_F_USER_STACK (1ULL << 8) + #define BPF_F_FAST_STACK_CMP (1ULL << 9) + #define BPF_F_REUSE_STACKID (1ULL << 10) + /* user accessible mirror of in-kernel sk_buff. * new fields can only be added to the end of this structure */
Re: linux-next: manual merge of the net-next tree with the net tree
On 02/26/2016 01:13 AM, Stephen Rothwell wrote: [...] I fixed it up (see below) and can carry the fix as necessary (no action is required). Looks good to me, thanks Stephen! Best, Daniel
Re: [PATCH net] ipv4: only create late gso-skb if skb is already set up with CHECKSUM_PARTIAL
Wakko Warner wrote: > Hannes Frederic Sowa wrote: > > Otherwise we break the contract with GSO to only pass CHECKSUM_PARTIAL > > skbs down. This can easily happen with UDP+IPv4 sockets with the first > > MSG_MORE write smaller than the MTU, second write is a sendfile. > > > > Returning -EOPNOTSUPP lets the callers fall back into normal sendmsg path, > > were we calculate the checksum manually during copying. > > > > Commit d749c9cbffd6 ("ipv4: no CHECKSUM_PARTIAL on MSG_MORE corked > > sockets") started to exposes this bug. > > > > Fixes: d749c9cbffd6 ("ipv4: no CHECKSUM_PARTIAL on MSG_MORE corked sockets") > > Reported-by: Jiri Benc > > Cc: Jiri Benc > > Reported-by: Wakko Warner > > Cc: Wakko Warner > > Signed-off-by: Hannes Frederic Sowa > > I just tested this with 2 of my VMs. It appears to have fixed the issue. Now there's another one: [ 777.315931] [ cut here ] [ 777.316099] WARNING: CPU: 0 PID: 1404 at /usr/src/linux/dist/4.4-nobklcd/net/ipv4/af_inet.c:155 inet_sock_destruct+0x1cb/0x1f0() [ 777.316189] Modules linked in: nfsv3 af_packet scsi_transport_iscsi nfsd auth_rpcgss oid_registry exportfs nfs lockd grace sunrpc ipv6 ata_piix libata evdev virtio_balloon virtio_net unix [ 777.316416] CPU: 0 PID: 1404 Comm: kworker/0:1H Not tainted 4.4.0 #2 [ 777.316468] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Debian-1.8.2-1 04/01/2014 [ 777.316547] Workqueue: rpciod xprt_autoclose [sunrpc] [ 777.316598] 815053f0 811d46ee 81041383 [ 777.316680] 88003d268b40 88003d268cc0 88003d07b3f8 88003d07b368 [ 777.316763] 8136f53b 88003d268b40 8800160f2d40 [ 777.316845] Call Trace: [ 777.316868] [] ? dump_stack+0x47/0x69 [ 777.316911] [] ? warn_slowpath_common+0x73/0xa0 [ 777.316963] [] ? inet_sock_destruct+0x1cb/0x1f0 [ 777.317018] [] ? sk_destruct+0x13/0xc0 [ 777.317061] [] ? inet_release+0x31/0x50 [ 777.317108] [] ? sock_release+0x15/0x70 [ 777.317153] [] ? xs_close+0x9/0x20 [sunrpc] [ 777.317206] [] ? xprt_autoclose+0x2d/0x60 [sunrpc] [ 777.317261] [] ? process_one_work+0x129/0x3f0 [ 777.317313] [] ? worker_thread+0x42/0x490 [ 777.317367] [] ? process_one_work+0x3f0/0x3f0 [ 777.317421] [] ? kthread+0xb8/0xd0 [ 777.317465] [] ? kthread_worker_fn+0x100/0x100 [ 777.317521] [] ? ret_from_fork+0x3f/0x70 [ 777.317564] [] ? kthread_worker_fn+0x100/0x100 [ 777.317618] ---[ end trace 220e17a0bf3ec971 ]--- This one happened on the client side VM. There was only 1 NFS mount. The server VM didn't show anything nor did the host. > > --- > > net/ipv4/ip_output.c | 5 - > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c > > index 64878efa045c13..565bf64b2b7d60 100644 > > --- a/net/ipv4/ip_output.c > > +++ b/net/ipv4/ip_output.c > > @@ -1236,13 +1236,16 @@ ssize_t ip_append_page(struct sock *sk, struct > > flowi4 *fl4, struct page *page, > > if (!skb) > > return -EINVAL; > > > > - cork->length += size; > > if ((size + skb->len > mtu) && > > (sk->sk_protocol == IPPROTO_UDP) && > > (rt->dst.dev->features & NETIF_F_UFO)) { > > + if (skb->ip_summed != CHECKSUM_PARTIAL) > > + return -EOPNOTSUPP; > > + > > skb_shinfo(skb)->gso_size = mtu - fragheaderlen; > > skb_shinfo(skb)->gso_type = SKB_GSO_UDP; > > } > > + cork->length += size; > > > > while (size > 0) { > > if (skb_is_gso(skb)) { > > -- > > 2.5.0 > > > -- > Microsoft has beaten Volkswagen's world record. Volkswagen only created 22 > million bugs. -- Microsoft has beaten Volkswagen's world record. Volkswagen only created 22 million bugs.
Re: [PATCH] appletalk: Pass IP-over-DDP packets through when 'ipddp0' interface is not present
On Thu, 2016-02-25 at 14:33 -0500, David Miller wrote: > From: Adam Seering > Date: Tue, 23 Feb 2016 09:19:13 -0500 > > > Let userspace programs transmit and receive raw IP-over-DDP packets > > with a kernel where "ipddp" was compiled as a module but is not > loaded > > (so no "ipddp0" network interface is exposed). This makes the > "module > > is not loaded" behavior match the "module was never compiled" > behavior. > > > > Signed-off-by: Adam Seering > > I think a better approache is to somehow autoload the module. Could you elaborate? Specifically: the kernel currently suppresses packets on behalf of the module even after the module is unloaded. How would autoloading the module help with that?
Re: [PATCH v2 net] bonding: don't use stale speed and duplex information
On 02/25/2016 09:33 PM, Jay Vosburgh wrote: zhuyj wrote: [...] I delved into the source code and Emil's tests. I think that the problem that this patch expects to fix occurs very unusually. Do you agree with me? If so, maybe the following patch can reduce the performance loss. Please comment on it. Thanks a lot. diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index b7f1a99..c4c511a 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -2129,7 +2129,9 @@ static void bond_miimon_commit(struct bonding *bond) continue; case BOND_LINK_UP: - bond_update_speed_duplex(slave); + if (slave->speed == SPEED_UNKNOWN) + bond_update_speed_duplex(slave); + bond_set_slave_link_state(slave, BOND_LINK_UP, BOND_SLAVE_NOTIFY_NOW); slave->last_link_up = jiffies; I don't believe the speed is necessarily SPEED_UNKNOWN coming in here. If the race occurs at a time later than the initial enslavement, speed may already be set (and the race manifests if the new speed changes, i.e., the link changes from 1 Gb/sec to 10 Gb/sec), so I don't think this is functionally correct. Hi, Jay Thanks for your reply. IMHO, "If the race occurs at a time later than the initial enslavement, speed may already be set (and the race manifests if the new speed changes, i.e., the link changes from 1 Gb/sec to 10 Gb/sec)", from my test, this will not happen because the previous source code make the speed correct. This "bond_update_speed_duplex" repeats to get the correct speed. That is, this patch is to fix the error in initial enslavement. The mentioned scenario will not occur. Even though the performance impact is minimal, if we can avoid this performance impact, why not ? Best Regards! Zhu Yanjun Also, the call to bond_miimon_commit itself is already gated by bond_miimon_inspect finding a link state change. The performance impact here should be minimal. -J --- -Jay Vosburgh, jay.vosbu...@canonical.com
[PATCH iproute2 v2] iplink: Support VF Trust
From: Hiroshi Shimamoto Add IFLA_VF_TRUST message to trust the VF. PF can accept some privileged operation from the trusted VF. For example, ixgbe PF doesn't allow to enable VF promiscuous mode until the VF is trusted because it may hurt performance. To trust VF. # ip link set dev eth0 vf 1 trust on To untrust VF. # ip link set dev eth0 vf 1 trust off Signed-off-by: Hiroshi Shimamoto --- v1 -> v2: rebase to the latest code of iproute2. The VF trust patch has been in kernel and the IFLA_VF_TRUST netlink attribute has been included iproute2, but no actual handler for this. This patch add the functionality to trust vf from ip command. ip/iplink.c | 13 + man/man8/ip-link.8.in | 7 ++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/ip/iplink.c b/ip/iplink.c index 5ab9d61..69f5057 100644 --- a/ip/iplink.c +++ b/ip/iplink.c @@ -82,6 +82,7 @@ void iplink_usage(void) fprintf(stderr, " [ spoofchk { on | off} ] ]\n"); fprintf(stderr, " [ query_rss { on | off} ] ]\n"); fprintf(stderr, " [ state { auto | enable | disable} ] ]\n"); + fprintf(stderr, " [ trust { on | off} ] ]\n"); fprintf(stderr, " [ master DEVICE ]\n"); fprintf(stderr, " [ nomaster ]\n"); fprintf(stderr, " [ addrgenmode { eui64 | none | stable_secret | random } ]\n"); @@ -356,6 +357,18 @@ static int iplink_parse_vf(int vf, int *argcp, char ***argvp, ivs.vf = vf; addattr_l(&req->n, sizeof(*req), IFLA_VF_RSS_QUERY_EN, &ivs, sizeof(ivs)); + } else if (matches(*argv, "trust") == 0) { + struct ifla_vf_trust ivt; + NEXT_ARG(); + if (matches(*argv, "on") == 0) + ivt.setting = 1; + else if (matches(*argv, "off") == 0) + ivt.setting = 0; + else + invarg("Invalid \"trust\" value\n", *argv); + ivt.vf = vf; + addattr_l(&req->n, sizeof(*req), IFLA_VF_TRUST, &ivt, sizeof(ivt)); + } else if (matches(*argv, "state") == 0) { struct ifla_vf_link_state ivl; diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in index 4d32343..7dd7a90 100644 --- a/man/man8/ip-link.8.in +++ b/man/man8/ip-link.8.in @@ -142,7 +142,8 @@ ip-link \- network device configuration .B min_tx_rate .IR TXRATE " ] [" .B spoofchk { on | off } ] [ -.B state { auto | enable | disable} +.B state { auto | enable | disable} ] [ +.B trust { on | off } ] | .br .B master @@ -1019,6 +1020,10 @@ parameter must be specified. reflection of the PF link state, enable lets the VF to communicate with other VFs on this host even if the PF link state is down, disable causes the HW to drop any packets sent by the VF. +.sp +.BI trust " on|off" +- trust the specified VF user. This enables that VF user can set a specific feature +which may impact security and/or performance. (e.g. VF multicast promiscuous mode) .in -8 .TP -- 1.8.3.1
Re: [PATCH net-next v7] net: ipv6: Make address flushing on ifdown optional
From: David Ahern Date: Wed, 24 Feb 2016 09:25:37 -0800 > Currently, all ipv6 addresses are flushed when the interface is configured > down, including global, static addresses: ... > Add a new sysctl to make this behavior optional. The new setting defaults to > flush all addresses to maintain backwards compatibility. When the set global > addresses with no expire times are not flushed on an admin down. The sysctl > is per-interface or system-wide for all interfaces > > $ sysctl -w net.ipv6.conf.eth1.keep_addr_on_down=1 > or > $ sysctl -w net.ipv6.conf.all.keep_addr_on_down=1 > > Will keep addresses on eth1 on an admin down. ... > Signed-off-by: David Ahern Applied, thanks for your persistence David :)
Re: [PATCH net-next] net: Facility to report route quality of connected sockets
From: Tom Herbert Date: Wed, 24 Feb 2016 10:02:52 -0800 > This patch add the SO_CNX_ADVICE socket option (setsockopt only). The > purpose is to allow an application to give feedback to the kernel about > the quality of the network path for a connected socket. The value > argument indicates the type of quality report. For this initial patch > the only supported advice is a value of 1 which indicates "bad path, > please reroute"-- the action taken by the kernel is to call > dst_negative_advice which will attempt to choose a different ECMP route, > reset the TX hash for flow label and UDP source port in encapsulation, > etc. > > This facility should be useful for connected UDP sockets where only the > application can provide any feedback about path quality. It could also > be useful for TCP applications that have additional knowledge about the > path outside of the normal TCP control loop. > > Signed-off-by: Tom Herbert This looks fine, applied, thanks Tom.
[net-next PATCH] IPv6: Use a 16 bit length field when computing a IPv6 UDP checksum
This change makes it so that we only use a 16 bit length field instead of a 32 bit length field when computing a UDP checksum for IPv6. This fixes an issue found with UDP tunnels over IPv6 where the total size exceeded 65536 for a frame that was to be segmented. As a result the checksum being computed didn't match the frame data so we ended up being off by 1 for the final checksum value since we didn't cancel out the upper 16 bits of the length. The reasoning behind this is that RFC2460 states that for protocols such as UDP that carry their own length field we should use that when computing the checksum for the pseudo-header. As such we should be using a 16 bit value, not a 32 bit as is currently occurring when computing the UDP checksum for IPv6. Signed-off-by: Alexander Duyck --- I don't have the highest of confidence that this patch is the correct solution for this, however the way I see it we only have a few alternatives. For now I am using this patch to get the conversation started. There ends up really being two issues here. The first issue is that IPv4 and IPv6 GSO frames are being assembled that have a length that is greater than 65535 and the truncated value is being placed in the length fields. The problem as I see it is that I am not sure there is any definitive way to prevent this without reducing the maximum GSO size for all sources within a given network namespace. In addition I am not sure what the impact on performance would be if we were to implement such an approach. The second issue is that if we cannot prevent the first issue we really need to have a consistent way of doing the IPv4 and IPv6 checksums. The IPv4 checksum passes the length value as a short, while the IPv6 passes this value as an integer. As a result we get different behavior between the two which makes it difficult for us to put together a generic routine for handling checksums for segmentation at higher levels. include/net/ip6_checksum.h |9 + 1 file changed, 9 insertions(+) diff --git a/include/net/ip6_checksum.h b/include/net/ip6_checksum.h index 1a49b73f7f6e..c55529ac12cf 100644 --- a/include/net/ip6_checksum.h +++ b/include/net/ip6_checksum.h @@ -95,6 +95,15 @@ static inline __sum16 udp_v6_check(int len, const struct in6_addr *daddr, __wsum base) { + /* RFC 2460 states that for protocols such as UDP which include +* a length in their header we should use that value when computing +* the pseudo-header instead of the total payload length minus +* extension headers. Since UDP has a length field that is only +* 16 bits in length we need to cap the length field to 16 bits to +* match what will be present in the header. +*/ + len &= 0x; + return csum_ipv6_magic(saddr, daddr, len, IPPROTO_UDP, base); }
Re: [PATCH net-next RFC] rtnetlink: add new RTM_GETSTATS to dump link stats
On 2/25/16, 8:26 AM, David Miller wrote: > From: roopa > Date: Wed, 24 Feb 2016 21:25:50 -0800 > >> I did go back and forth on the attribute vs mask. >> cosmetic but, i guess i did not feel good about having to redefine every >> attribute again >> for the bitmap filter ...and i anticipate the list of stat attributes to >> grow overtime (maybe there is a better way). >> enum { >>IFLA_LINK_STATS64, >>IFLA_LINK_INET6_STATS, >>IFLA_LINK_MPLS_STATS, >>__IFLA_LINK_STATS_MAX, >> }; >> >> #define IFLA_LINK_STATS64_FILTER 0 >> #define IFLA_LINK_INET6_STATS_FILTER (1 << 0) >> #define IFLA_LINK_MPLS_STATS_FILTER (2 << 0) > The filter for X is always (1 << X), so we could work with something like: > > #define IFLA_LINK_FILTER_BIT(ATTR)(1 << (ATTR)) i like it > > Which seems to suggest that emitting no stats unless they are explicitly > requested in > the bitmask makes sense because: > > 1) You don't have to special case STATS64 in the filter mask > > 2) Application are forced to be aware of filtering and thus choose only >what they want to see > > How about this? I am ok with it. It keeps the filtering mask handling consistent. Its a bit inconsistent with the other dump functions, but this gives user full control of what combination of stats she wants. For something like stats, i think this makes sense. Thanks again! Roopa
Re: [PATCH 1/4] kernel: time: Add current_nw_timestamp() for network timestamps
Hi, Deepa Dinamani wrote: >>> include/linux/ip.h | 2 ++ >>> include/linux/time64.h | 3 +++ >>> kernel/time/time.c | 26 ++ >>> 3 files changed, 31 insertions(+) >>> >> Since net/ipv4/* are the only users, it is enough to put >> it in under net/ipv4/. > > time.c hosts functions that are used by individual subsystems like > current_fs_time() used by filesystems > (sometimes used by other subsystems also). > > The network timestamp function is used for both source route ip option > and timestamp icmp messages. > So it makes it difficult for it to be owned by a single layer. > This is the reason it was chosen to include here. > > Another option is to include it in the lowest layer its used: > af_inet.c. Is this what you were suggesting? > Yes, that's right. --yoshfuji > -Deepa >
Re: [PATCH] ipv4: do not cache local route when using SO_BINDTODEVICE
Thanks for reviewing. David Miller writes: > From: Kouya Shimura > Date: Tue, 23 Feb 2016 14:27:32 +0900 > >> After v3.6, output routes are cached, however, the 'rt_iif' field of >> struct rtable can not be shared by various traffics using SO_BINDTODEVICE. >> It causes that traffic can not reach the local receiver which also uses >> SO_BINDTODEVICE after another traffic creates a local route cache. >> >> /* sender and receiver are running on same host. */ >> sender: >> - socket(SOCK_DGRAM) >> - setsockopt(SO_BINDTODEVICE, "eth0") >> - connect(INADDR_ANY) >> - send("message") -->[loopback]--+ >> | >> receiver: | >> - socket(SOCK_DGRAM) | >> - setsockopt(SO_BINDTODEVICE, "eth0")| >> - bind(INADDR_ANY) | >> - recv() <--+ >> /* sometimes reach, sometimes not reach!!! */ >> >> Before v3.6, above traffics always reached. It seems to be a >> regression. Actually the 'dhcp_release' command relies on it. >> >> Commit 7bd86cc282a458b66c41e3f6 ("ipv4: Cache local output routes") >> originated this issue. Revert it conditionally. >> >> Signed-off-by: Kouya Shimura > > I strongly fear that this will cause a performance regression for some > important > cases. If you elide the cache, it means that every route lookup has > to allocate, > create, and destroy the cache object. This is really expensive. The performace degradation was on my mind. Thus, I intended that this patch affects only special (abnormal?) case that destination is local and using SO_BINDTODEVICE. Isn't it acceptable? This patch deesn't elide the existing cache. Normal (not using SO_BINDTODEVICE) routes are still cached (not destoyed). Outer routes, too. I confirmed it. If you worry about non-zeroed 'orig_oif' case, this patch certainly implies a perfomance problem. 'orig_oif' seems to be non-zero in cases of ICMP, GRE, MULTICAST besides SO_BINDTODEVICE. > > There has to be a better way to do so, so I'm not applying this patch, > sorry. Yes, it has to be. The root cause is sharing the 'rt_iif' field. I'm looking forward to the right fix. Thanks, Kouya
Re: [net-next PATCH v2 3/3] net: sched: cls_u32 add bit to specify software only rules
On 2/25/2016 3:20 PM, John Fastabend wrote: In the initial implementation the only way to stop a rule from being inserted into the hardware table was via the device feature flag. However this doesn't work well when working on an end host system where packets are expect to hit both the hardware and software datapaths. For example we can imagine a rule that will match an IP address and increment a field. If we install this rule in both hardware and software we may increment the field twice. To date we have only added support for the drop action so we have been able to ignore these cases. But as we extend the action support we will hit this example plus more such cases. Arguably these are not even corner cases in many working systems these cases will be common. To avoid forcing the driver to always abort (i.e. the above example) this patch adds a flag to add a rule in software only. A careful user can use this flag to build software and hardware datapaths that work together. One example we have found particularly useful is to use hardware resources to set the skb->mark on the skb when the match may be expensive to run in software but a mark lookup in a hash table is cheap. The idea here is hardware can do in one lookup what the u32 classifier may need to traverse multiple lists and hash tables to compute. The flag is only passed down on inserts on deletion to avoid stale references in hardware we always try I think this is supposed to be a new sentence starting with 'On deletion' to remove a rule if it exists. The flags field is part of the classifier specific options. Although it is tempting to lift this into the generic structure doing this proves difficult do to how the tc netlink attributes are implemented along with how the dump/change routines are called. There is also precedence for putting seemingly generic pieces in the specific classifier options such as TCA_U32_POLICE, TCA_U32_ACT, etc. So although not ideal I've left FLAGS in the u32 options as well as it simplifies the code greatly and user space has already learned how to manage these bits ala 'tc' tool. Another thing if trying to update a rule we require the flags to be unchanged. This is to force user space, software u32 and the hardware u32 to keep in sync. Thanks to Simon Horman for catching this case. Signed-off-by: John Fastabend --- include/net/pkt_cls.h| 13 +++-- include/uapi/linux/pkt_cls.h |1 + net/sched/cls_u32.c | 37 +++-- 3 files changed, 39 insertions(+), 12 deletions(-) @@ -482,7 +485,9 @@ static void u32_clear_hw_hnode(struct tcf_proto *tp, struct tc_u_hnode *h) } } -static void u32_replace_hw_knode(struct tcf_proto *tp, struct tc_u_knode *n) +static void u32_replace_hw_knode(struct tcf_proto *tp, +struct tc_u_knode *n, +u32 flags) { struct net_device *dev = tp->q->dev_queue->dev; struct tc_cls_u32_offload u32_offload = {0}; @@ -491,7 +496,7 @@ static void u32_replace_hw_knode(struct tcf_proto *tp, struct tc_u_knode *n) offload.type = TC_SETUP_CLSU32; offload.cls_u32 = &u32_offload; - if (tc_should_offload(dev)) { + if (tc_should_offload(dev, flags)) { offload.cls_u32->command = TC_CLSU32_REPLACE_KNODE; offload.cls_u32->knode.handle = n->handle; offload.cls_u32->knode.fshift = n->fshift; @@ -679,6 +684,7 @@ static const struct nla_policy u32_policy[TCA_U32_MAX + 1] = { [TCA_U32_SEL] = { .len = sizeof(struct tc_u32_sel) }, [TCA_U32_INDEV] = { .type = NLA_STRING, .len = IFNAMSIZ }, [TCA_U32_MARK] = { .len = sizeof(struct tc_u32_mark) }, + [TCA_U32_FLAGS] = { .len = NLA_U32 }, should be .type = NLA_U32
Re: header conflict introduced by change to netfilter_ipv4/ip_tables.h
On Thu, Feb 25, 2016 at 09:53:32PM +0100, Daniel Borkmann wrote: > On 02/04/2016 08:13 AM, Josh Boyer wrote: > >On Thu, Jan 7, 2016 at 2:15 PM, Mikko Rapeli wrote: > >>On Thu, Jan 07, 2016 at 10:30:40AM -0800, Stephen Hemminger wrote: > >>>On Thu, 7 Jan 2016 07:29:50 + > >>>Mikko Rapeli wrote: > On Wed, Jan 06, 2016 at 09:20:07AM -0800, Stephen Hemminger wrote: > >This commit breaks compilation of iproute2 with net-next. > > Ok, linux/if.h and libc net/if.h have overlapping defines, and this is not > the only one. I saw lots of them in the core dump headers. > > How should we handle them? Another ifndef for IFNAMSIZ into kernel uapi > headers? > > -Mikko > >>> > >>>Probably need to do the same thing that was done previously for these > >>>kind of conflicts. This makes make linux/if.h change to adapt to net/if.h > >>>being included before it. > >> > >>Ok, got it. And found include/uapi/linux/libc-compat.h. Did not know about > >>it > >>and was looking for solutions to these problems. > >> > >>But now I feel like writing a test script for mixing of kernel uapi > >>and libc headers to find out how many other collitions are still there. > >>Not good for the pile of over 70 patches in my branch > >>https://github.com/torvalds/linux/compare/master...mcfrisk:headers_test_v05 > >> > >>>Or revert your patch. > >> > >>I'm fine with this too. > > > >This is causing a number of build failures in Fedora rawhide now. Did > >anyone submit a revert or patch to fix this issue? > > Mikko, was there any follow-up patch to fix this? Seems like the build > error is not yet resolved. First draft of proper fix: https://lkml.org/lkml/2016/2/7/89 Will fix review comments from David Miller soon and I need to figure out how to get the needed fixes to the glibc side too. The problem from my point of view is that kernel uapi header files have incomplete dependencies, which were hiding kernel uapi and glibc header file conflicts. I will try to systematically fix the kernel uapi and glibc header conflicts but this will take some time. If you want, you can hide the problem by reverting the needed header file dependency fixes. -Mikko
Re: header conflict introduced by change to netfilter_ipv4/ip_tables.h
On Thu, Feb 25, 2016 at 10:08:56PM +0100, Thomas Graf wrote: > On 01/06/16 at 09:20am, Stephen Hemminger wrote: > > This commit breaks compilation of iproute2 with net-next. > > > > commit 1ffad83dffd675cd742286ae82dca7d746cb0da8 > > Author: Mikko Rapeli > > Date: Thu Oct 15 07:56:30 2015 +0200 > > > > netfilter: fix include files for compilation > > > > Add missing header dependencies and other small changes so that each > > file > > compiles alone in userspace. > > > > Signed-off-by: Mikko Rapeli > > Signed-off-by: Pablo Neira Ayuso > > > > For iproute2, a copy of kernel headers (from make install_headers) is used. > > After this change. the build of x_tables.c fails because IFNAMSIZ is already > > defined in net/if.h > > There is another issue with this commit. iptables.h included from m_ipt.c > includes xtables.h which includes which is not > available on a system without xtables. > > gcc -Wall -Wstrict-prototypes -Wmissing-prototypes -Wmissing-declarations > -Wold-style-definition -Wformat=2 -O2 -I../include -DRESOLVE_HOSTNAMES > -DLIBDIR=\"/usr/lib\" -DCONFDIR=\"/etc/iproute2\" -D_GNU_SOURCE > -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE > -DHAVE_SETNS -DHAVE_ELF -DCONFIG_GACT -DCONFIG_GACT_PROB > -DIPT_LIB_DIR=\"/lib/xtables\" -DYY_NO_INPUT -c -o m_ipt.o m_ipt.c > In file included from ../include/iptables.h:5:0, > from m_ipt.c:17: > ../include/xtables.h:34:29: fatal error: xtables-version.h: No such file or > directory > #include I don't see any dependencies from kernel side uapi headers to iptables or xtables in my tree based on v4.5-rc2. Maybe this is a problem in iproute2. -Mikko
Re: [net-next PATCH v2 3/3] net: sched: cls_u32 add bit to specify software only rules
On 16-02-25 11:02 PM, Samudrala, Sridhar wrote: > On 2/25/2016 3:20 PM, John Fastabend wrote: >> In the initial implementation the only way to stop a rule from being >> inserted into the hardware table was via the device feature flag. >> However this doesn't work well when working on an end host system >> where packets are expect to hit both the hardware and software >> datapaths. >> >> For example we can imagine a rule that will match an IP address and >> increment a field. If we install this rule in both hardware and >> software we may increment the field twice. To date we have only >> added support for the drop action so we have been able to ignore >> these cases. But as we extend the action support we will hit this >> example plus more such cases. Arguably these are not even corner >> cases in many working systems these cases will be common. >> >> To avoid forcing the driver to always abort (i.e. the above example) >> this patch adds a flag to add a rule in software only. A careful >> user can use this flag to build software and hardware datapaths >> that work together. One example we have found particularly useful >> is to use hardware resources to set the skb->mark on the skb when >> the match may be expensive to run in software but a mark lookup >> in a hash table is cheap. The idea here is hardware can do in one >> lookup what the u32 classifier may need to traverse multiple lists >> and hash tables to compute. The flag is only passed down on inserts >> on deletion to avoid stale references in hardware we always try > > I think this is supposed to be a new sentence starting with 'On deletion' Yep. >> to remove a rule if it exists. >> >> The flags field is part of the classifier specific options. Although >> it is tempting to lift this into the generic structure doing this >> proves difficult do to how the tc netlink attributes are implemented >> along with how the dump/change routines are called. There is also >> precedence for putting seemingly generic pieces in the specific >> classifier options such as TCA_U32_POLICE, TCA_U32_ACT, etc. So >> although not ideal I've left FLAGS in the u32 options as well as it >> simplifies the code greatly and user space has already learned how >> to manage these bits ala 'tc' tool. >> >> Another thing if trying to update a rule we require the flags to >> be unchanged. This is to force user space, software u32 and >> the hardware u32 to keep in sync. Thanks to Simon Horman for >> catching this case. >> [...] >> u32_policy[TCA_U32_MAX + 1] = { >> [TCA_U32_SEL]= { .len = sizeof(struct tc_u32_sel) }, >> [TCA_U32_INDEV]= { .type = NLA_STRING, .len = IFNAMSIZ }, >> [TCA_U32_MARK]= { .len = sizeof(struct tc_u32_mark) }, >> +[TCA_U32_FLAGS]= { .len = NLA_U32 }, > should be .type = NLA_U32 > Yep stupid typo. I think I'm going to write some smatch files to catch these sorts of things they should be detectable pragmatically. Thanks. >> >
Re: [PATCH net] ipv4: only create late gso-skb if skb is already set up with CHECKSUM_PARTIAL
On 26.02.2016 01:45, Wakko Warner wrote: Now there's another one: [ 777.315931] [ cut here ] [ 777.316099] WARNING: CPU: 0 PID: 1404 at /usr/src/linux/dist/4.4-nobklcd/net/ipv4/af_inet.c:155 inet_sock_destruct+0x1cb/0x1f0() [ 777.316189] Modules linked in: nfsv3 af_packet scsi_transport_iscsi nfsd auth_rpcgss oid_registry exportfs nfs lockd grace sunrpc ipv6 ata_piix libata evdev virtio_balloon virtio_net unix [ 777.316416] CPU: 0 PID: 1404 Comm: kworker/0:1H Not tainted 4.4.0 #2 [ 777.316468] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Debian-1.8.2-1 04/01/2014 [ 777.316547] Workqueue: rpciod xprt_autoclose [sunrpc] [ 777.316598] 815053f0 811d46ee 81041383 [ 777.316680] 88003d268b40 88003d268cc0 88003d07b3f8 88003d07b368 [ 777.316763] 8136f53b 88003d268b40 8800160f2d40 [ 777.316845] Call Trace: [ 777.316868] [] ? dump_stack+0x47/0x69 [ 777.316911] [] ? warn_slowpath_common+0x73/0xa0 [ 777.316963] [] ? inet_sock_destruct+0x1cb/0x1f0 [ 777.317018] [] ? sk_destruct+0x13/0xc0 [ 777.317061] [] ? inet_release+0x31/0x50 [ 777.317108] [] ? sock_release+0x15/0x70 [ 777.317153] [] ? xs_close+0x9/0x20 [sunrpc] [ 777.317206] [] ? xprt_autoclose+0x2d/0x60 [sunrpc] [ 777.317261] [] ? process_one_work+0x129/0x3f0 [ 777.317313] [] ? worker_thread+0x42/0x490 [ 777.317367] [] ? process_one_work+0x3f0/0x3f0 [ 777.317421] [] ? kthread+0xb8/0xd0 [ 777.317465] [] ? kthread_worker_fn+0x100/0x100 [ 777.317521] [] ? ret_from_fork+0x3f/0x70 [ 777.317564] [] ? kthread_worker_fn+0x100/0x100 [ 777.317618] ---[ end trace 220e17a0bf3ec971 ]--- This one happened on the client side VM. There was only 1 NFS mount. The server VM didn't show anything nor did the host. Can you send me your specific kernel version? There are multiple WARN_ONs and I want to catch the right one. Thanks!
Re: [PATCH] uapi glibc compat: fix cases where glibc net/if.h is included before linux/if.h
(Adding libc-alpha list, review of https://lkml.org/lkml/2016/2/7/89 ) On Wed, Feb 17, 2016 at 10:46:20AM -0500, David Miller wrote: > From: Mikko Rapeli > Date: Sun, 7 Feb 2016 16:03:21 +0200 > > > @@ -68,6 +72,8 @@ > > * @IFF_ECHO: echo sent packets. Volatile. > > */ > > enum net_device_flags { > > +/* for compatibility with glibc net/if.h */ > > +#if __UAPI_DEF_IF_NET_DEVICE_FLAGS > > IFF_UP = 1<<0, /* sysfs */ > > IFF_BROADCAST = 1<<1, /* volatile */ > > IFF_DEBUG = 1<<2, /* sysfs */ > > @@ -84,11 +90,14 @@ enum net_device_flags { > > IFF_PORTSEL = 1<<13, /* sysfs */ > > IFF_AUTOMEDIA = 1<<14, /* sysfs */ > > IFF_DYNAMIC = 1<<15, /* sysfs */ > > +#endif /* __UAPI_DEF_IF_NET_DEVICE_FLAGS */ > > IFF_LOWER_UP= 1<<16, /* volatile */ > > IFF_DORMANT = 1<<17, /* volatile */ > > IFF_ECHO= 1<<18, /* volatile */ > > }; > > This is going to get messy is IFF_LOWER_UP, IFF_DORMANT, and IFF_ECHO > get added the the glibc header. Why not just handle it now with > another __UAPI_DEF_FOO guard so that the additions to net/if.h can > deal with this case too. Do you mean that the enum should be protected with a single guard or should I have one guard for current conflicts and one for the future if glibc headers include IFF_LOWER_UP and others? -Mikko
pull-request: can 2016-02-26
Hello David, this is a pull request of one patch for net. The patch by Maximilain Schneider fixes a kfree() problem during disconnect in the gs_usb driver. regrds, Marc --- The following changes since commit 4c0b6eaf373a5323f03a3a20c42fc435715b073d: net: thunderx: Fix for Qset error due to CQ full (2016-02-25 16:25:34 -0500) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can.git tags/linux-can-fixes-for-4.5-20160226 for you to fetch changes up to e9a2d81b1761093386a0bb8a4f51642ac785ef63: can: gs_usb: fixed disconnect bug by removing erroneous use of kfree() (2016-02-26 08:36:33 +0100) linux-can-fixes-for-4.5-20160226 Maximilain Schneider (1): can: gs_usb: fixed disconnect bug by removing erroneous use of kfree() drivers/net/can/usb/gs_usb.c | 24 +++- 1 file changed, 11 insertions(+), 13 deletions(-)
[PATCH] can: gs_usb: fixed disconnect bug by removing erroneous use of kfree()
From: Maximilain Schneider gs_destroy_candev() erroneously calls kfree() on a struct gs_can *, which is allocated through alloc_candev() and should instead be freed using free_candev() alone. The inappropriate use of kfree() causes the kernel to hang when gs_destroy_candev() is called. Only the struct gs_usb * which is allocated through kzalloc() should be freed using kfree() when the device is disconnected. Signed-off-by: Maximilian Schneider Cc: linux-stable Signed-off-by: Marc Kleine-Budde --- drivers/net/can/usb/gs_usb.c | 24 +++- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c index 5eee62badf45..cbc99d5649af 100644 --- a/drivers/net/can/usb/gs_usb.c +++ b/drivers/net/can/usb/gs_usb.c @@ -826,9 +826,8 @@ static struct gs_can *gs_make_candev(unsigned int channel, struct usb_interface static void gs_destroy_candev(struct gs_can *dev) { unregister_candev(dev->netdev); - free_candev(dev->netdev); usb_kill_anchored_urbs(&dev->tx_submitted); - kfree(dev); + free_candev(dev->netdev); } static int gs_usb_probe(struct usb_interface *intf, const struct usb_device_id *id) @@ -913,12 +912,15 @@ static int gs_usb_probe(struct usb_interface *intf, const struct usb_device_id * for (i = 0; i < icount; i++) { dev->canch[i] = gs_make_candev(i, intf); if (IS_ERR_OR_NULL(dev->canch[i])) { + /* save error code to return later */ + rc = PTR_ERR(dev->canch[i]); + /* on failure destroy previously created candevs */ icount = i; - for (i = 0; i < icount; i++) { + for (i = 0; i < icount; i++) gs_destroy_candev(dev->canch[i]); - dev->canch[i] = NULL; - } + + usb_kill_anchored_urbs(&dev->rx_submitted); kfree(dev); return rc; } @@ -939,16 +941,12 @@ static void gs_usb_disconnect(struct usb_interface *intf) return; } - for (i = 0; i < GS_MAX_INTF; i++) { - struct gs_can *can = dev->canch[i]; - - if (!can) - continue; - - gs_destroy_candev(can); - } + for (i = 0; i < GS_MAX_INTF; i++) + if (dev->canch[i]) + gs_destroy_candev(dev->canch[i]); usb_kill_anchored_urbs(&dev->rx_submitted); + kfree(dev); } static const struct usb_device_id gs_usb_table[] = { -- 2.7.0
[PATCH net-next 3/5] vxlan: move fdb code to common location in vxlan_xmit
Handle VXLAN_F_COLLECT_METADATA before VXLAN_F_PROXY. The latter does not make sense with the former, as it needs populated fdb which does not happen in metadata mode. After this cleanup, the fdb code in vxlan_xmit is moved to a common location and can be later skipped for L3 mode vxlan tunnels. Signed-off-by: Jiri Benc --- drivers/net/vxlan.c | 22 +++--- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c index 49fcca8d8a8e..029e0e5b0b2b 100644 --- a/drivers/net/vxlan.c +++ b/drivers/net/vxlan.c @@ -2139,9 +2139,17 @@ static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev) info = skb_tunnel_info(skb); skb_reset_mac_header(skb); - eth = eth_hdr(skb); - if ((vxlan->flags & VXLAN_F_PROXY)) { + if (vxlan->flags & VXLAN_F_COLLECT_METADATA) { + if (info && info->mode & IP_TUNNEL_INFO_TX) + vxlan_xmit_one(skb, dev, NULL, false); + else + kfree_skb(skb); + return NETDEV_TX_OK; + } + + if (vxlan->flags & VXLAN_F_PROXY) { + eth = eth_hdr(skb); if (ntohs(eth->h_proto) == ETH_P_ARP) return arp_reduce(dev, skb); #if IS_ENABLED(CONFIG_IPV6) @@ -2156,18 +2164,10 @@ static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev) msg->icmph.icmp6_type == NDISC_NEIGHBOUR_SOLICITATION) return neigh_reduce(dev, skb); } - eth = eth_hdr(skb); #endif } - if (vxlan->flags & VXLAN_F_COLLECT_METADATA) { - if (info && info->mode & IP_TUNNEL_INFO_TX) - vxlan_xmit_one(skb, dev, NULL, false); - else - kfree_skb(skb); - return NETDEV_TX_OK; - } - + eth = eth_hdr(skb); f = vxlan_find_mac(vxlan, eth->h_dest); did_rsc = false; -- 1.8.3.1
[PATCH net-next 2/5] vxlan: move L2 mode initialization to a separate function
This will allow to initialize vxlan in L3 mode based on the passed rtnl attributes. Signed-off-by: Jiri Benc --- drivers/net/vxlan.c | 20 +--- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c index c7844bae339d..49fcca8d8a8e 100644 --- a/drivers/net/vxlan.c +++ b/drivers/net/vxlan.c @@ -2437,7 +2437,7 @@ static int vxlan_fill_metadata_dst(struct net_device *dev, struct sk_buff *skb) return 0; } -static const struct net_device_ops vxlan_netdev_ops = { +static const struct net_device_ops vxlan_netdev_l2mode_ops = { .ndo_init = vxlan_init, .ndo_uninit = vxlan_uninit, .ndo_open = vxlan_open, @@ -2491,10 +2491,6 @@ static void vxlan_setup(struct net_device *dev) struct vxlan_dev *vxlan = netdev_priv(dev); unsigned int h; - eth_hw_addr_random(dev); - ether_setup(dev); - - dev->netdev_ops = &vxlan_netdev_ops; dev->destructor = free_netdev; SET_NETDEV_DEVTYPE(dev, &vxlan_type); @@ -2509,8 +2505,7 @@ static void vxlan_setup(struct net_device *dev) dev->hw_features |= NETIF_F_GSO_SOFTWARE; dev->hw_features |= NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_STAG_TX; netif_keep_dst(dev); - dev->priv_flags &= ~IFF_TX_SKB_SHARING; - dev->priv_flags |= IFF_LIVE_ADDR_CHANGE | IFF_NO_QUEUE; + dev->priv_flags |= IFF_NO_QUEUE; INIT_LIST_HEAD(&vxlan->next); spin_lock_init(&vxlan->hash_lock); @@ -2529,6 +2524,15 @@ static void vxlan_setup(struct net_device *dev) INIT_HLIST_HEAD(&vxlan->fdb_head[h]); } +static void vxlan_l2mode_setup(struct net_device *dev) +{ + eth_hw_addr_random(dev); + ether_setup(dev); + dev->priv_flags &= ~IFF_TX_SKB_SHARING; + dev->priv_flags |= IFF_LIVE_ADDR_CHANGE; + dev->netdev_ops = &vxlan_netdev_l2mode_ops; +} + static const struct nla_policy vxlan_policy[IFLA_VXLAN_MAX + 1] = { [IFLA_VXLAN_ID] = { .type = NLA_U32 }, [IFLA_VXLAN_GROUP] = { .len = FIELD_SIZEOF(struct iphdr, daddr) }, @@ -2759,6 +2763,8 @@ static int vxlan_dev_configure(struct net *src_net, struct net_device *dev, ((conf->flags & VXLAN_F_GBP) && (conf->flags & VXLAN_F_GPE))) return -EINVAL; + vxlan_l2mode_setup(dev); + vxlan->net = src_net; dst->remote_vni = conf->vni; -- 1.8.3.1
[PATCH net-next 4/5] vxlan: fix too large pskb_may_pull with remote checksum
The vxlan header is pulled at this point, don't include it again in the calculation. Signed-off-by: Jiri Benc --- drivers/net/vxlan.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c index 029e0e5b0b2b..81a7c1a829b9 100644 --- a/drivers/net/vxlan.c +++ b/drivers/net/vxlan.c @@ -1142,7 +1142,7 @@ static int vxlan_igmp_leave(struct vxlan_dev *vxlan) static bool vxlan_remcsum(struct vxlanhdr *unparsed, struct sk_buff *skb, u32 vxflags) { - size_t start, offset, plen; + size_t start, offset; if (!(unparsed->vx_flags & VXLAN_HF_RCO) || skb->remcsum_offload) goto out; @@ -1150,9 +1150,7 @@ static bool vxlan_remcsum(struct vxlanhdr *unparsed, start = vxlan_rco_start(unparsed->vx_vni); offset = start + vxlan_rco_offset(unparsed->vx_vni); - plen = sizeof(struct vxlanhdr) + offset + sizeof(u16); - - if (!pskb_may_pull(skb, plen)) + if (!pskb_may_pull(skb, offset + sizeof(u16))) return false; skb_remcsum_process(skb, (void *)(vxlan_hdr(skb) + 1), start, offset, -- 1.8.3.1
[PATCH net-next 1/5] vxlan: implement GPE in L2 mode
Implement VXLAN-GPE. Only L2 mode (i.e. encapsulated Ethernet frame) is supported by this patch. L3 mode will be added by subsequent patches. Signed-off-by: Jiri Benc --- drivers/net/vxlan.c | 68 ++-- include/net/vxlan.h | 62 +++- include/uapi/linux/if_link.h | 8 ++ 3 files changed, 135 insertions(+), 3 deletions(-) diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c index 775ddb48388d..c7844bae339d 100644 --- a/drivers/net/vxlan.c +++ b/drivers/net/vxlan.c @@ -1192,6 +1192,33 @@ out: unparsed->vx_flags &= ~VXLAN_GBP_USED_BITS; } +static bool vxlan_parse_gpe_hdr(struct vxlanhdr *unparsed, + struct sk_buff *skb, u32 vxflags) +{ + struct vxlanhdr_gpe *gpe = (struct vxlanhdr_gpe *)unparsed; + + /* Need to have Next Protocol set for interfaces in GPE mode. */ + if (!gpe->np_applied) + return false; + /* "The initial version is 0. If a receiver does not support the +* version indicated it MUST drop the packet. +*/ + if (gpe->version != 0) + return false; + /* "When the O bit is set to 1, the packet is an OAM packet and OAM +* processing MUST occur." However, we don't implement OAM +* processing, thus drop the packet. +*/ + if (gpe->oam_flag) + return false; + + if (gpe->next_protocol != VXLAN_GPE_NP_ETHERNET) + return false; + + unparsed->vx_flags &= ~VXLAN_GPE_USED_BITS; + return true; +} + static bool vxlan_set_mac(struct vxlan_dev *vxlan, struct vxlan_sock *vs, struct sk_buff *skb) @@ -1307,6 +1334,9 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb) /* For backwards compatibility, only allow reserved fields to be * used by VXLAN extensions if explicitly requested. */ + if (vs->flags & VXLAN_F_GPE) + if (!vxlan_parse_gpe_hdr(&unparsed, skb, vs->flags)) + goto drop; if (vs->flags & VXLAN_F_REMCSUM_RX) if (!vxlan_remcsum(&unparsed, skb, vs->flags)) goto drop; @@ -1685,6 +1715,14 @@ static void vxlan_build_gbp_hdr(struct vxlanhdr *vxh, u32 vxflags, gbp->policy_id = htons(md->gbp & VXLAN_GBP_ID_MASK); } +static void vxlan_build_gpe_hdr(struct vxlanhdr *vxh, u32 vxflags) +{ + struct vxlanhdr_gpe *gpe = (struct vxlanhdr_gpe *)vxh; + + gpe->np_applied = 1; + gpe->next_protocol = VXLAN_GPE_NP_ETHERNET; +} + static int vxlan_build_skb(struct sk_buff *skb, struct dst_entry *dst, int iphdr_len, __be32 vni, struct vxlan_metadata *md, u32 vxflags, @@ -1744,6 +1782,8 @@ static int vxlan_build_skb(struct sk_buff *skb, struct dst_entry *dst, if (vxflags & VXLAN_F_GBP) vxlan_build_gbp_hdr(vxh, vxflags, md); + if (vxflags & VXLAN_F_GPE) + vxlan_build_gpe_hdr(vxh, vxflags); skb_set_inner_protocol(skb, htons(ETH_P_TEB)); return 0; @@ -2515,6 +2555,7 @@ static const struct nla_policy vxlan_policy[IFLA_VXLAN_MAX + 1] = { [IFLA_VXLAN_REMCSUM_RX] = { .type = NLA_U8 }, [IFLA_VXLAN_GBP]= { .type = NLA_FLAG, }, [IFLA_VXLAN_REMCSUM_NOPARTIAL] = { .type = NLA_FLAG }, + [IFLA_VXLAN_GPE_MODE] = { .type = NLA_U8, }, }; static int vxlan_validate(struct nlattr *tb[], struct nlattr *data[]) @@ -2714,6 +2755,10 @@ static int vxlan_dev_configure(struct net *src_net, struct net_device *dev, __be16 default_port = vxlan->cfg.dst_port; struct net_device *lowerdev = NULL; + if (((conf->flags & VXLAN_F_LEARN) && (conf->flags & VXLAN_F_GPE)) || + ((conf->flags & VXLAN_F_GBP) && (conf->flags & VXLAN_F_GPE))) + return -EINVAL; + vxlan->net = src_net; dst->remote_vni = conf->vni; @@ -2770,8 +2815,12 @@ static int vxlan_dev_configure(struct net *src_net, struct net_device *dev, dev->needed_headroom = needed_headroom; memcpy(&vxlan->cfg, conf, sizeof(*conf)); - if (!vxlan->cfg.dst_port) - vxlan->cfg.dst_port = default_port; + if (!vxlan->cfg.dst_port) { + if (conf->flags & VXLAN_F_GPE) + vxlan->cfg.dst_port = 4790; /* IANA assigned VXLAN-GPE port */ + else + vxlan->cfg.dst_port = default_port; + } vxlan->flags |= conf->flags; if (!vxlan->cfg.age_interval) @@ -2941,6 +2990,13 @@ static int vxlan_newlink(struct net *src_net, struct net_device *dev, if (data[IFLA_VXLAN_REMCSUM_NOPARTIAL]) conf.flags |= VXLAN_F_REMCSUM_NOPARTIAL; + if (data[IFLA_VXLAN_GPE_MODE]) { + u8 mode = nla_get_u8(data[IFLA_VXLAN_GPE_MODE]); + + if (mod
[PATCH net-next 0/5] vxlan: implement Generic Protocol Extension (GPE)
VXLAN-GPE can operate in two modes: with encapsulated Ethernet header (L2 mode) or with L3 header (e.g. IP header) directly following VXLAN-GPE header (L3 mode). Add support for both modes. The L2 mode is simple, as it's basically the same as plain VXLAN, only with added bits in the header. The L3 mode is more complicated. The patches adding it follow the same model as the tun/tap driver: depending on the chosen mode, the vxlan interface is created either as ARPHRD_ETHER (L2 mode) or ARPHRD_NONE (L3 mode). Note that the internal fdb control plane cannot be used together with VXLAN-GPE and attempt to configure it will be rejected by the driver. This in theory could be relaxed for L2 mode in the future if such need arises. Jiri Benc (5): vxlan: implement GPE in L2 mode vxlan: move L2 mode initialization to a separate function vxlan: move fdb code to common location in vxlan_xmit vxlan: fix too large pskb_may_pull with remote checksum vxlan: implement GPE in L3 mode drivers/net/vxlan.c | 223 --- include/net/vxlan.h | 63 +++- include/uapi/linux/if_link.h | 9 ++ 3 files changed, 258 insertions(+), 37 deletions(-) -- 1.8.3.1
[PATCH net-next 5/5] vxlan: implement GPE in L3 mode
Implement L3 mode for VXLAN-GPE (i.e. IPv4/IPv6 payload directly after the VXLAN header). The GPE header parsing has to be moved before iptunnel_pull_header, as we need to know the protocol. Signed-off-by: Jiri Benc --- drivers/net/vxlan.c | 127 +++ include/net/vxlan.h | 3 +- include/uapi/linux/if_link.h | 1 + 3 files changed, 108 insertions(+), 23 deletions(-) diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c index 81a7c1a829b9..b79472d3fd36 100644 --- a/drivers/net/vxlan.c +++ b/drivers/net/vxlan.c @@ -1191,6 +1191,7 @@ out: } static bool vxlan_parse_gpe_hdr(struct vxlanhdr *unparsed, + __be32 *protocol, struct sk_buff *skb, u32 vxflags) { struct vxlanhdr_gpe *gpe = (struct vxlanhdr_gpe *)unparsed; @@ -1210,9 +1211,30 @@ static bool vxlan_parse_gpe_hdr(struct vxlanhdr *unparsed, if (gpe->oam_flag) return false; - if (gpe->next_protocol != VXLAN_GPE_NP_ETHERNET) + /* L2 mode */ + if (gpe->next_protocol == VXLAN_GPE_NP_ETHERNET) { + if (vxflags & VXLAN_F_GPE_L3) + return false; + *protocol = htons(ETH_P_TEB); + goto out; + } + + /* L3 mode */ + if (!(vxflags & VXLAN_F_GPE_L3)) + return false; + + switch (gpe->next_protocol) { + case VXLAN_GPE_NP_IPV4: + *protocol = htons(ETH_P_IP); + break; + case VXLAN_GPE_NP_IPV6: + *protocol = htons(ETH_P_IPV6); + break; + default: return false; + } +out: unparsed->vx_flags &= ~VXLAN_GPE_USED_BITS; return true; } @@ -1282,9 +1304,10 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb) struct vxlanhdr unparsed; struct vxlan_metadata _md; struct vxlan_metadata *md = &_md; + __be32 protocol = htons(ETH_P_TEB); void *oiph; - /* Need Vxlan and inner Ethernet header to be present */ + /* Need UDP and VXLAN header to be present */ if (!pskb_may_pull(skb, VXLAN_HLEN)) return 1; @@ -1308,7 +1331,14 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb) if (!vxlan) goto drop; - if (iptunnel_pull_header(skb, VXLAN_HLEN, htons(ETH_P_TEB), + /* For backwards compatibility, only allow reserved fields to be +* used by VXLAN extensions if explicitly requested. +*/ + if (vs->flags & VXLAN_F_GPE) + if (!vxlan_parse_gpe_hdr(&unparsed, &protocol, skb, vs->flags)) + goto drop; + + if (iptunnel_pull_header(skb, VXLAN_HLEN, protocol, !net_eq(vxlan->net, dev_net(vxlan->dev goto drop; @@ -1329,12 +1359,6 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb) memset(md, 0, sizeof(*md)); } - /* For backwards compatibility, only allow reserved fields to be -* used by VXLAN extensions if explicitly requested. -*/ - if (vs->flags & VXLAN_F_GPE) - if (!vxlan_parse_gpe_hdr(&unparsed, skb, vs->flags)) - goto drop; if (vs->flags & VXLAN_F_REMCSUM_RX) if (!vxlan_remcsum(&unparsed, skb, vs->flags)) goto drop; @@ -1353,8 +1377,13 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb) goto drop; } - if (!vxlan_set_mac(vxlan, vs, skb)) - goto drop; + if (protocol == htons(ETH_P_TEB)) { + if (!vxlan_set_mac(vxlan, vs, skb)) + goto drop; + } else { + skb->dev = vxlan->dev; + skb->pkt_type = PACKET_HOST; + } oiph = skb_network_header(skb); skb_reset_network_header(skb); @@ -1713,12 +1742,29 @@ static void vxlan_build_gbp_hdr(struct vxlanhdr *vxh, u32 vxflags, gbp->policy_id = htons(md->gbp & VXLAN_GBP_ID_MASK); } -static void vxlan_build_gpe_hdr(struct vxlanhdr *vxh, u32 vxflags) +static int vxlan_build_gpe_hdr(struct vxlanhdr *vxh, u32 vxflags, + __be16 protocol) { struct vxlanhdr_gpe *gpe = (struct vxlanhdr_gpe *)vxh; gpe->np_applied = 1; - gpe->next_protocol = VXLAN_GPE_NP_ETHERNET; + + /* L2 mode */ + if (!(vxflags & VXLAN_F_GPE_L3)) { + gpe->next_protocol = VXLAN_GPE_NP_ETHERNET; + return 0; + } + + /* L3 mode */ + switch (protocol) { + case htons(ETH_P_IP): + gpe->next_protocol = VXLAN_GPE_NP_IPV4; + return 0; + case htons(ETH_P_IPV6): + gpe->next_protocol = VXLAN_GPE_NP_IPV6; + return 0; + } + return -EPFNOSUPPORT; } static int vxlan_build_skb(struct sk_
pull-request: can-next 2016-02-26
Hello David, this is a pull request of 3 patch for net-next/master. There are two patches by Simon Horman, in which the device tree support for the rcar_can driver is improved. One patch by me fixes the bad coding style of the ems_usb driver which was introduced recently. regards, Marc --- The following changes since commit 8d3f2806f8fbd9b222b3504580b5eaa9ba2964b8: Merge branch 'ethtool-ksettings' (2016-02-25 22:07:00 -0500) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git tags/linux-can-next-for-4.6-20160226 for you to fetch changes up to f71096dfd129e4ad1ae80cd10d5ac050e5730f8a: can: rcar: add device tree support for r8a779[234] (2016-02-26 08:43:34 +0100) linux-can-next-for-4.6-20160226 Marc Kleine-Budde (1): can: ems_usb: fix coding style Simon Horman (2): can: rcar: add gen[12] fallback compatibility strings can: rcar: add device tree support for r8a779[234] Documentation/devicetree/bindings/net/can/rcar_can.txt | 11 ++- drivers/net/can/rcar_can.c | 2 ++ drivers/net/can/usb/ems_usb.c | 8 +++- 3 files changed, 15 insertions(+), 6 deletions(-) -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature