Netdev List
 help / color / mirror / Atom feed
* Re: [net-next PATCH 2/3] net/qdisc: IFF_NO_QUEUE drivers should use consistent TX queue len
From: Krister Johansen @ 2016-11-03 20:54 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: netdev, Phil Sutter, Robert Olsson, Jamal Hadi Salim
In-Reply-To: <20161103135606.28737.67383.stgit@firesoul>

On Thu, Nov 03, 2016 at 02:56:06PM +0100, Jesper Dangaard Brouer wrote:
> The flag IFF_NO_QUEUE marks virtual device drivers that doesn't need a
> default qdisc attached, given they will be backed by physical device,
> that already have a qdisc attached for pushback.
> 
> It is still supported to attach a qdisc to a IFF_NO_QUEUE device, as
> this can be useful for difference policy reasons (e.g. bandwidth
> limiting containers).  For this to work, the tx_queue_len need to have
> a sane value, because some qdiscs inherit/copy the tx_queue_len
> (namely, pfifo, bfifo, gred, htb, plug and sfb).
> 
> Commit a813104d9233 ("IFF_NO_QUEUE: Fix for drivers not calling
> ether_setup()") caught situations where some drivers didn't initialize
> tx_queue_len.  The problem with the commit was choosing 1 as the
> fallback value.
> 
> A qdisc queue length of 1 causes more harm than good, because it
> creates hard to debug situations for userspace. It gives userspace a
> false sense of a working config after attaching a qdisc.  As low
> volume traffic (that doesn't activate the qdisc policy) works,
> like ping, while traffic that e.g. needs shaping cannot reach the
> configured policy levels, given the queue length is too small.

Thanks for fixing this.  I've run into this in the exact scenario you
describe -- bandwith limiting containers.  I'm pretty sure my vote
doesn't count, but I'm in favor of this change.

-K

^ permalink raw reply

* Re: [Patch net] genetlink: fix a memory leak on error path
From: David Miller @ 2016-11-03 20:53 UTC (permalink / raw)
  To: xiyou.wangcong; +Cc: netdev, kubakici, johannes
In-Reply-To: <1478191356-10386-1-git-send-email-xiyou.wangcong@gmail.com>

From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Thu,  3 Nov 2016 09:42:35 -0700

> In __genl_register_family(), when genl_validate_assign_mc_groups()
> fails, we forget to free the memory we possibly allocate for
> family->attrbuf.
> 
> Note, some callers call genl_unregister_family() to clean up
> on error path, it doesn't work because the family is inserted
> to the global list in the nearly last step.
> 
> Cc: Jakub Kicinski <kubakici@wp.pl>
> Cc: Johannes Berg <johannes@sipsolutions.net>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>

Applied, thanks Cong.

^ permalink raw reply

* Re: [PATCH net] ipv6: dccp: add missing bind_conflict to dccp_ipv6_mapped
From: David Miller @ 2016-11-03 20:51 UTC (permalink / raw)
  To: eric.dumazet; +Cc: andreyknvl, netdev, syzkaller, gerrit, dccp, acme
In-Reply-To: <1478188786.7065.453.camel@edumazet-glaptop3.roam.corp.google.com>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 03 Nov 2016 08:59:46 -0700

> From: Eric Dumazet <edumazet@google.com>
> 
> While fuzzing kernel with syzkaller, Andrey reported a nasty crash
> in inet6_bind() caused by DCCP lacking a required method.
> 
> Fixes: ab1e0a13d7029 ("[SOCK] proto: Add hashinfo member to struct proto")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Andrey Konovalov <andreyknvl@google.com>
> Tested-by: Andrey Konovalov <andreyknvl@google.com>

Applied and queued up for -stable, thanks Eric.

^ permalink raw reply

* Re: [RFC] make kmemleak scan __ro_after_init section (was: Re: [PATCH 0/5] genetlink improvements)
From: Catalin Marinas @ 2016-11-03 20:49 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Cong Wang, Johannes Berg, Linux Kernel Network Developers, LKML,
	linux-mm
In-Reply-To: <20161102234755.4381f528@jkicinski-Precision-T1700>

On Wed, Nov 02, 2016 at 11:47:55PM +0000, Jakub Kicinski wrote:
> I realized that kmemleak is not scanning the __ro_after_init section...
> Following patch solves the false positives but I wonder if it's the
> right/acceptable solution.

Thanks for putting this together. I actually hit a similar issue on
arm64 but didn't get the chance to fix it (also at LPC). With a proper
commit message, feel free to add:

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [PATCH net v3] ipv4: allow local fragmentation in ip_finish_output_gso()
From: Shmulik Ladkani @ 2016-11-03 20:40 UTC (permalink / raw)
  To: David Miller; +Cc: lrichard, netdev, fw, jtluka, hannes
In-Reply-To: <20161103.161244.1518109748163260273.davem@davemloft.net>

On Thu, 03 Nov 2016 16:12:44 -0400 (EDT) David Miller <davem@davemloft.net> wrote:
> Applied and queued up for -stable.

Dave, my response lagged your "Applied" by few minutes ;)

This seems to deserve some more thought to make sure nothing got broken,
as expressed last in https://patchwork.ozlabs.org/patch/690594/

Best,
Shmulik

^ permalink raw reply

* Re: Time to revisit LISP?
From: David Miller @ 2016-11-03 20:37 UTC (permalink / raw)
  To: tom; +Cc: chris, netdev
In-Reply-To: <CALx6S35cT6gn4W6iiaL5sAgTm=jJPZpyVP3pyGXfbyHHsKaciQ@mail.gmail.com>

From: Tom Herbert <tom@herbertland.com>
Date: Thu, 3 Nov 2016 12:22:52 -0700

> For instance, one of the his questions is:
> 
> "What is to keep one from having to service a full Map-Request -->
> Map-Reply cycle for every packet received?"
> 
> This can be solved by judicious rate limiting, for instance the
> infrastructure I implemented to rate limit ILA resolver request could
> be applied here.

All of these things work great if you have tables that are either very
tiny or change infrequently.

But once you run into anything seriously dynamic, it has the same
problems that the routing cache had and OVS can have.

And for this reason things like the flow cache are on the chopping
block.  And frankly, I'd remove OVS from the kernel if I could.

Userspace resolution of paths in response to data path signalling
simply does not scale and is fundamentally an extremely poor design
choice.  We're trying to move away from, rather than towards, these
kinds of architectures.

^ permalink raw reply

* Re: [PATCH net v3] ipv4: allow local fragmentation in ip_finish_output_gso()
From: Shmulik Ladkani @ 2016-11-03 20:27 UTC (permalink / raw)
  To: Lance Richardson, fw, hannes; +Cc: netdev, jtluka
In-Reply-To: <1478118977-19608-1-git-send-email-lrichard@redhat.com>

Hi Hannes, Lance,

On Wed,  2 Nov 2016 16:36:17 -0400 Lance Richardson <lrichard@redhat.com> wrote:
>  
> -	if (skb_iif && !(df & htons(IP_DF))) {
> -		/* Arrived from an ingress interface, got encapsulated, with
> -		 * fragmentation of encapulating frames allowed.
> -		 * If skb is gso, the resulting encapsulated network segments
> -		 * may exceed dst mtu.
> -		 * Allow IP Fragmentation of segments.
> -		 */
> -		IPCB(skb)->flags |= IPSKB_FRAG_SEGS;
> -	}

Thinking this over, I'm concerned of this change.

Few months back, we discussed this and got to the conclusion that in the
"ingress,tunnel,egress" scenario, segments are allowed to be
fragmented if the original inner ip packet does NOT have the DF.

See 
  https://patchwork.ozlabs.org/patch/657132/
  https://patchwork.ozlabs.org/patch/661219/

I think you expressed that those tunneled skbs already having DF set
should go through pmtu discovery.

Suggested patch unconditionally calls skb_gso_validate_mtu().

Thus we're changing behavior for "ingress,tunnel,egress" scenario of
the tunneled packets having DF set in the inner iph.

WDYT?

^ permalink raw reply

* Re: [PATCH net-next] net/sched: cls_flower: Support matching on SCTP ports
From: David Miller @ 2016-11-03 20:26 UTC (permalink / raw)
  To: simon.horman; +Cc: netdev, jiri
In-Reply-To: <1478175862-28642-1-git-send-email-simon.horman@netronome.com>

From: Simon Horman <simon.horman@netronome.com>
Date: Thu,  3 Nov 2016 13:24:21 +0100

> Support matching on SCTP ports in the same way that matching
> on TCP and UDP ports is already supported.
> 
> Example usage:
> 
> tc qdisc add dev eth0 ingress
> 
> tc filter add dev eth0 protocol ip parent ffff: \
>         flower indev eth0 ip_proto sctp dst_port 80 \
>         action drop
> 
> Signed-off-by: Simon Horman <simon.horman@netronome.com>

Applied, thanks Simon.

^ permalink raw reply

* Re: [PATCH net] ehea: fix operation state report
From: David Miller @ 2016-11-03 20:26 UTC (permalink / raw)
  To: gpiccoli; +Cc: netdev, dougmill, zhou
In-Reply-To: <1478168180-18371-1-git-send-email-gpiccoli@linux.vnet.ibm.com>

From: "Guilherme G. Piccoli" <gpiccoli@linux.vnet.ibm.com>
Date: Thu,  3 Nov 2016 08:16:20 -0200

> Currently the ehea driver is missing a call to netif_carrier_off()
> before the interface bring-up; this is necessary in order to
> initialize the __LINK_STATE_NOCARRIER bit in the net_device state
> field. Otherwise, we observe state UNKNOWN on "ip address" command
> output.
> 
> This patch adds a call to netif_carrier_off() on ehea's net device
> open callback.
> 
> Reported-by: Xiong Zhou <zhou@redhat.com>
> Reference-ID: IBM bz #137702, Red Hat bz #1089134
> Signed-off-by: Guilherme G. Piccoli <gpiccoli@linux.vnet.ibm.com>

Applied.

^ permalink raw reply

* Re: [patch net-next] mlxsw: pci: Fix the FW ready mask length
From: David Miller @ 2016-11-03 20:21 UTC (permalink / raw)
  To: jiri; +Cc: netdev, eladr
In-Reply-To: <1478162515-2910-1-git-send-email-jiri@resnulli.us>

From: Jiri Pirko <jiri@resnulli.us>
Date: Thu,  3 Nov 2016 09:41:55 +0100

> From: Elad Raz <eladr@mellanox.com>
> 
> The system-status register is actually 16-bit wide and not 8 bit-wide.
> 
> Fixes: 233fa44bd67ae ("mlxsw: pci: Implement reset done check")
> Signed-off-by: Elad Raz <eladr@mellanox.com>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>

Applied.

^ permalink raw reply

* Re: [PATCH net] ipv6: dccp: fix out of bound access in dccp_v6_err()
From: David Miller @ 2016-11-03 20:20 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev, gerrit, dccp
In-Reply-To: <1478143848.7065.429.camel@edumazet-glaptop3.roam.corp.google.com>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 02 Nov 2016 20:30:48 -0700

> From: Eric Dumazet <edumazet@google.com>
> 
> dccp_v6_err() does not use pskb_may_pull() and might access garbage.
> 
> We only need 4 bytes at the beginning of the DCCP header, like TCP,
> so the 8 bytes pulled in icmpv6_notify() are more than enough.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Applied and queue up for -stable.

Thanks Eric.

^ permalink raw reply

* Re: [PATCH net] netlink: netlink_diag_dump() runs without locks
From: David Miller @ 2016-11-03 20:20 UTC (permalink / raw)
  To: eric.dumazet
  Cc: andreyknvl, herbert, akpm, decot, dmitrijs.ivanovs, edumazet, fw,
	grose, johannes.berg, matti.vaittinen, pshelar, stephen, tom,
	tycho.andersen, linux-kernel, netdev, syzkaller, kcc, glider,
	dvyukov
In-Reply-To: <1478143280.7065.427.camel@edumazet-glaptop3.roam.corp.google.com>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 02 Nov 2016 20:21:20 -0700

> From: Eric Dumazet <edumazet@google.com>
> 
> A recent commit removed locking from netlink_diag_dump() but forgot
> one error case.
 ...
> Fixes: ad202074320c ("netlink: Use rhashtable walk interface in diag dump")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Andrey Konovalov <andreyknvl@google.com>
> Tested-by: Andrey Konovalov <andreyknvl@google.com>

Applied.

^ permalink raw reply

* Re: [PATCH net] dccp: fix out of bound access in dccp_v4_err()
From: David Miller @ 2016-11-03 20:20 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev, gerrit, dccp
In-Reply-To: <1478138440.7065.419.camel@edumazet-glaptop3.roam.corp.google.com>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 02 Nov 2016 19:00:40 -0700

> From: Eric Dumazet <edumazet@google.com>
> 
> dccp_v4_err() does not use pskb_may_pull() and might access garbage.
> 
> We only need 4 bytes at the beginning of the DCCP header, like TCP,
> so the 8 bytes pulled in icmp_socket_deliver() are more than enough.
> 
> This patch might allow to process more ICMP messages, as some routers
> are still limiting the size of reflected bytes to 28 (RFC 792), instead
> of extended lengths (RFC 1812 4.3.2.3)
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Applied and queued up for -stable.

^ permalink raw reply

* Re: [PATCH net] dccp: do not send reset to already closed sockets
From: David Miller @ 2016-11-03 20:19 UTC (permalink / raw)
  To: eric.dumazet
  Cc: andreyknvl, gerrit, dccp, netdev, syzkaller, kcc, glider,
	edumazet, dvyukov
In-Reply-To: <1478135064.7065.409.camel@edumazet-glaptop3.roam.corp.google.com>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 02 Nov 2016 18:04:24 -0700

> From: Eric Dumazet <edumazet@google.com>
> 
> Andrey reported following warning while fuzzing with syzkaller
> 
> WARNING: CPU: 1 PID: 21072 at net/dccp/proto.c:83 dccp_set_state+0x229/0x290
> Kernel panic - not syncing: panic_on_warn set ...
 ...
> Fix this the same way we did for TCP in commit 565b7b2d2e63
> ("tcp: do not send reset to already closed sockets")
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Andrey Konovalov <andreyknvl@google.com>
> Tested-by: Andrey Konovalov <andreyknvl@google.com>

Applied and queued up for -stable.

^ permalink raw reply

* Re: [PATCH net] dccp: do not release listeners too soon
From: David Miller @ 2016-11-03 20:19 UTC (permalink / raw)
  To: eric.dumazet
  Cc: andreyknvl, gerrit, dccp, netdev, dvyukov, glider, kcc, edumazet,
	syzkaller
In-Reply-To: <1478132081.7065.403.camel@edumazet-glaptop3.roam.corp.google.com>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 02 Nov 2016 17:14:41 -0700

> From: Eric Dumazet <edumazet@google.com>
> 
> Andrey Konovalov reported following error while fuzzing with syzkaller :
 ...
> It turns out DCCP calls __sk_receive_skb(), and this broke when
> lookups no longer took a reference on listeners.
> 
> Fix this issue by adding a @refcounted parameter to __sk_receive_skb(),
> so that sock_put() is used only when needed.
> 
> Fixes: 3b24d854cb35 ("tcp/dccp: do not touch listener sk_refcnt under synflood")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Andrey Konovalov <andreyknvl@google.com>
> Tested-by: Andrey Konovalov <andreyknvl@google.com>

Applied and queued up for -stable.

^ permalink raw reply

* Re: [PATCH net] tcp: fix return value for partial writes
From: David Miller @ 2016-11-03 20:12 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev, soheil, willemb, ycheng, ncardwell
In-Reply-To: <1478122910.7065.396.camel@edumazet-glaptop3.roam.corp.google.com>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 02 Nov 2016 14:41:50 -0700

> From: Eric Dumazet <edumazet@google.com>
> 
> After my commit, tcp_sendmsg() might restart its loop after
> processing socket backlog.
> 
> If sk_err is set, we blindly return an error, even though we
> copied data to user space before.
> 
> We should instead return number of bytes that could be copied,
> otherwise user space might resend data and corrupt the stream.
> 
> This might happen if another thread is using recvmsg(MSG_ERRQUEUE)
> to process timestamps.
> 
> Issue was diagnosed by Soheil and Willem, big kudos to them !
> 
> Fixes: d41a69f1d390f ("tcp: make tcp_sendmsg() aware of socket backlog")
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Applied and queued up for -stable.

^ permalink raw reply

* Re: [PATCH net v3] ipv4: allow local fragmentation in ip_finish_output_gso()
From: David Miller @ 2016-11-03 20:12 UTC (permalink / raw)
  To: lrichard; +Cc: netdev, fw, jtluka, hannes
In-Reply-To: <1478118977-19608-1-git-send-email-lrichard@redhat.com>

From: Lance Richardson <lrichard@redhat.com>
Date: Wed,  2 Nov 2016 16:36:17 -0400

> Some configurations (e.g. geneve interface with default
> MTU of 1500 over an ethernet interface with 1500 MTU) result
> in the transmission of packets that exceed the configured MTU.
> While this should be considered to be a "bad" configuration,
> it is still allowed and should not result in the sending
> of packets that exceed the configured MTU.
> 
> Fix by dropping the assumption in ip_finish_output_gso() that
> locally originated gso packets will never need fragmentation.
> Basic testing using iperf (observing CPU usage and bandwidth)
> have shown no measurable performance impact for traffic not
> requiring fragmentation.
> 
> Fixes: c7ba65d7b649 ("net: ip: push gso skb forwarding handling down the stack")
> Reported-by: Jan Tluka <jtluka@redhat.com>
> Signed-off-by: Lance Richardson <lrichard@redhat.com>

Applied and queued up for -stable.

^ permalink raw reply

* Re: [PATCH net-next] net: remove unused argument in checksum unnecessary conversion
From: David Miller @ 2016-11-03 20:08 UTC (permalink / raw)
  To: willemdebruijn.kernel; +Cc: netdev, tom, willemb
In-Reply-To: <1478117651-24909-1-git-send-email-willemdebruijn.kernel@gmail.com>

From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date: Wed,  2 Nov 2016 16:14:11 -0400

> From: Willem de Bruijn <willemb@google.com>
> 
> The check argument is never used. This code has not changed since
> the original introduction in d96535a17dbb ("net: Infrastructure for
> checksum unnecessary conversions"). Remove the unused argument and
> update all callers.
> 
> Signed-off-by: Willem de Bruijn <willemb@google.com>

Applied.

^ permalink raw reply

* Re: [PATCH] net: tcp: check skb is non-NULL for exact match on lookups
From: David Miller @ 2016-11-03 20:06 UTC (permalink / raw)
  To: dsa; +Cc: netdev, andreyknvl
In-Reply-To: <1478113705-16174-1-git-send-email-dsa@cumulusnetworks.com>

From: David Ahern <dsa@cumulusnetworks.com>
Date: Wed,  2 Nov 2016 12:08:25 -0700

> Andrey reported the following error report while running the syzkaller
> fuzzer:
 ...
> MD5 has a code path that calls __inet_lookup_listener with a null skb,
> so inet{6}_exact_dif_match needs to check skb against null before pulling
> the flag.
> 
> Fixes: a04a480d4392 ("net: Require exact match for TCP socket lookups if
>        dif is l3mdev")
> Reported-by: Andrey Konovalov <andreyknvl@google.com>
> Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
> ---
> Dave: commit a04a480d4392 was queued for stable, so this needs to follow it.

Applied and queued up for -stable, thanks.

^ permalink raw reply

* Re: [PATCH v5 0/7] add NS2 support to bgmac
From: David Miller @ 2016-11-03 20:02 UTC (permalink / raw)
  To: jon.mason-dY08KVG/lbpWk0Htik3J/w
  Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	f.fainelli-Re5JQEeQqe8AvxtiuMwx3w, rafal-g1n6cQUeyibVItvQsEIGlw,
	bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w,
	netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1478106488-11779-1-git-send-email-jon.mason-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>

From: Jon Mason <jon.mason-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
Date: Wed,  2 Nov 2016 13:08:01 -0400

> Add support for the amac found in the Broadcom Northstar2 SoC to the
> bgmac driver.  This necessitates adding support to connect to an
> externally defined phy (as described in the device tree) in the driver.
> These phy changes are in addition to the changes necessary to get NS2
> working.

This does not apply cleanly to the net-next, please respin.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH net-next v6 02/10] dpaa_eth: add support for DPAA Ethernet
From: David Miller @ 2016-11-03 19:58 UTC (permalink / raw)
  To: madalin.bucur
  Cc: netdev, linuxppc-dev, linux-kernel, oss, ppc, joe, pebolle,
	joakim.tjernlund
In-Reply-To: <1478117854-8952-3-git-send-email-madalin.bucur@nxp.com>

From: Madalin Bucur <madalin.bucur@nxp.com>
Date: Wed, 2 Nov 2016 22:17:26 +0200

> This introduces the Freescale Data Path Acceleration Architecture
> +static inline size_t bpool_buffer_raw_size(u8 index, u8 cnt)
> +{
> +	u8 i;
> +	size_t res = DPAA_BP_RAW_SIZE / 2;

Always order local variable declarations from longest to shortest line,
also know as Reverse Christmas Tree Format.

Please audit your entire submission for this problem, it occurs
everywhere.

> +	/* we do not want shared skbs on TX */
> +	net_dev->priv_flags &= ~IFF_TX_SKB_SHARING;

Why?  By clearing this, you disallow an important fundamental way to do
performane testing, via pktgen.


> +	int numstats = sizeof(struct rtnl_link_stats64) / sizeof(u64);
 ...
> +		cpustats = (u64 *)&percpu_priv->stats;
> +
> +		for (j = 0; j < numstats; j++)
> +			netstats[j] += cpustats[j];

This is a memcpy() on well-typed datastructures which requires no
casting or special handling whatsoever, so use memcpy instead of
needlessly open coding the operation.

> +static int dpaa_change_mtu(struct net_device *net_dev, int new_mtu)
> +{
> +	const int max_mtu = dpaa_get_max_mtu();
> +
> +	/* Make sure we don't exceed the Ethernet controller's MAXFRM */
> +	if (new_mtu < 68 || new_mtu > max_mtu) {
> +		netdev_err(net_dev, "Invalid L3 mtu %d (must be between %d and %d).\n",
> +			   new_mtu, 68, max_mtu);
> +		return -EINVAL;
> +	}
> +	net_dev->mtu = new_mtu;
> +
> +	return 0;
> +}

MTU restrictions are handled in the net-next tree via net_dev->min_mtu and
net_dev->max_mtu.  Use that and do not define this NDO operation as you do
not need it.

> +static int dpaa_set_features(struct net_device *dev, netdev_features_t features)
> +{
> +	/* Not much to do here for now */
> +	dev->features = features;
> +	return 0;
> +}

Do not define unnecessary NDO operations, let the defaults do their job.

> +static netdev_features_t dpaa_fix_features(struct net_device *dev,
> +					   netdev_features_t features)
> +{
> +	netdev_features_t unsupported_features = 0;
> +
> +	/* In theory we should never be requested to enable features that
> +	 * we didn't set in netdev->features and netdev->hw_features at probe
> +	 * time, but double check just to be on the safe side.
> +	 */
> +	unsupported_features |= NETIF_F_RXCSUM;
> +
> +	features &= ~unsupported_features;
> +
> +	return features;
> +}

Unless you can show that your need this, do not "guess" by implement this
NDO operation.  You don't need it.

> +#ifdef CONFIG_FSL_DPAA_ETH_FRIENDLY_IF_NAME
> +static int dpaa_mac_hw_index_get(struct platform_device *pdev)
> +{
> +	struct device *dpaa_dev;
> +	struct dpaa_eth_data *eth_data;
> +
> +	dpaa_dev = &pdev->dev;
> +	eth_data = dpaa_dev->platform_data;
> +
> +	return eth_data->mac_hw_id;
> +}
> +
> +static int dpaa_mac_fman_index_get(struct platform_device *pdev)
> +{
> +	struct device *dpaa_dev;
> +	struct dpaa_eth_data *eth_data;
> +
> +	dpaa_dev = &pdev->dev;
> +	eth_data = dpaa_dev->platform_data;
> +
> +	return eth_data->fman_hw_id;
> +}
> +#endif

Do not play network device naming games like this, use the standard name
assignment done by the kernel and have userspace entities do geographic or
device type specific naming.

I want to see this code completely removed.

> +static int dpaa_set_mac_address(struct net_device *net_dev, void *addr)
> +{
> +	const struct dpaa_priv	*priv;
> +	int err;
> +	struct mac_device *mac_dev;
> +
> +	priv = netdev_priv(net_dev);
> +
> +	err = eth_mac_addr(net_dev, addr);
> +	if (err < 0) {
> +		netif_err(priv, drv, net_dev, "eth_mac_addr() = %d\n", err);
> +		return err;
> +	}
> +
> +	mac_dev = priv->mac_dev;
> +
> +	err = mac_dev->change_addr(mac_dev->fman_mac,
> +				   (enet_addr_t *)net_dev->dev_addr);
> +	if (err < 0) {
> +		netif_err(priv, drv, net_dev, "mac_dev->change_addr() = %d\n",
> +			  err);
> +		return err;
> +	}

You MUST NOT return an error at this point without rewinding the state change
performed by eth_mac_addr().  Otherwise device will be left in an inconsistent
state compared to what the software MAC address has recorded.

This driver is enormous, I don't have the time nor the patience to
review it further for what seems to be many fundamental errors like
the ones I have pointed out so far.

Sorry.

^ permalink raw reply

* Re: [PATCH net-next 0/3] ip: add RECVFRAGSIZE cmsg
From: David Miller @ 2016-11-03 19:41 UTC (permalink / raw)
  To: willemdebruijn.kernel; +Cc: netdev, jdorfman, eric.dumazet, willemb
In-Reply-To: <1478098938-124108-1-git-send-email-willemdebruijn.kernel@gmail.com>

From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date: Wed,  2 Nov 2016 11:02:15 -0400

> On IP datagrams and raw sockets, when packets arrive fragmented,
> expose the largest received fragment size through a new cmsg.
> 
> Protocols implemented on top of these sockets may use this, for
> instance, to inform peers to lower MSS on platforms that silently
> allow send calls to exceed PMTU and cause fragmentation.

Looks good, series applied, thanks Willem.

^ permalink raw reply

* Re: [PATCH net] tcp: fix potential memory corruption
From: David Miller @ 2016-11-03 19:34 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev, hans.westgaard.ry, haakon.bugge
In-Reply-To: <1478098397.7065.370.camel@edumazet-glaptop3.roam.corp.google.com>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 02 Nov 2016 07:53:17 -0700

> From: Eric Dumazet <edumazet@google.com>
> 
> Imagine initial value of max_skb_frags is 17, and last
> skb in write queue has 15 frags.
> 
> Then max_skb_frags is lowered to 14 or smaller value.
> 
> tcp_sendmsg() will then be allowed to add additional page frags
> and eventually go past MAX_SKB_FRAGS, overflowing struct
> skb_shared_info.
> 
> Fixes: 5f74f82ea34c ("net:Add sysctl_max_skb_frags")
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Applied and queued up for -stable, thanks.

^ permalink raw reply

* Re: [PATCH net] qede: Correctly map aggregation replacement pages
From: David Miller @ 2016-11-03 19:34 UTC (permalink / raw)
  To: Yuval.Mintz; +Cc: netdev
In-Reply-To: <1478097406-9794-1-git-send-email-Yuval.Mintz@cavium.com>

From: Yuval Mintz <Yuval.Mintz@cavium.com>
Date: Wed, 2 Nov 2016 16:36:46 +0200

> Driver allocates replacement buffers before-hand to make
> sure whenever an aggregation begins there would be a replacement
> for the Rx buffers, as we can't release the buffer until
> aggregation is terminated and driver logic assumes the Rx rings
> are always full.
> 
> For every other Rx page that's being allocated [I.e., regular]
> the page is being completely mapped while for the replacement
> buffers only the first portion of the page is being mapped.
> This means that:
>   a. Once replacement buffer replenishes the regular Rx ring,
> assuming there's more than a single packet on page we'd post unmapped
> memory toward HW [assuming mapping is actually done in granularity
> smaller than page].
>   b. Unmaps are being done for the entire page, which is incorrect.
> 
> Fixes: 55482edc25f06 ("qede: Add slowpath/fastpath support and enable hardware GRO")
> Signed-off-by: Yuval Mintz <Yuval.Mintz@cavium.com>

Applied.

^ permalink raw reply

* Re: [PATCH v3 0/2] net: stmmac: Add OXNAS DWMAC Glue
From: David Miller @ 2016-11-03 19:31 UTC (permalink / raw)
  To: narmstrong
  Cc: peppe.cavallaro, alexandre.torgue, netdev, linux-oxnas,
	linux-arm-kernel, linux-kernel
In-Reply-To: <20161102140237.6955-1-narmstrong@baylibre.com>

From: Neil Armstrong <narmstrong@baylibre.com>
Date: Wed,  2 Nov 2016 15:02:35 +0100

> This patchset add support for the Sysnopsys DWMAC Gigabit Ethernet
> controller Glue layer of the Oxford Semiconductor OX820 SoC.

Series applied to net-next, thanks.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox