* [GIT] [4.6] NFC update
From: Samuel Ortiz @ 2016-03-11 20:48 UTC (permalink / raw)
To: David S. Miller; +Cc: Linux NFC, netdev
Hi David,
This is a very small one this time, with only 5 patches.
There are a couple of big items that could not be merged/finished
on time.
We have:
- 2 LLCP fixes for a race and a potential OOM.
- 2 cleanups for the pn544 and microread drivers.
- 1 Maintainer addition for the s3fwrn5 driver.
The following changes since commit 667f00630ebefc4d73aa105c6ab254e4aec867f8:
Merge branch 'local-checksum-offload' (2016-02-12 05:52:41 -0500)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/sameo/nfc-next.git tags/nfc-next-4.6-1
for you to fetch changes up to 079c2652e5af648db6bf4f54bcafdafcc57a0d2c:
MAINTAINERS: nfc: s3fwrn5: Add second maintainer (2016-03-10 17:16:22 +0100)
----------------------------------------------------------------
Cong Wang (2):
NFC: Use GFP_USER for user-controlled kmalloc
NFC: Close a race condition in llcp_sock_getname()
Jean Delvare (1):
NFC: microread: Drop platform data header file
Mika Westerberg (1):
NFC: pn544: Drop two useless checks in ACPI probe path
Robert Baldyga (1):
MAINTAINERS: nfc: s3fwrn5: Add second maintainer
MAINTAINERS | 2 +-
drivers/nfc/microread/i2c.c | 8 --------
drivers/nfc/pn544/i2c.c | 14 +------------
include/linux/platform_data/microread.h | 35 ---------------------------------
net/nfc/llcp_commands.c | 4 ++--
net/nfc/llcp_sock.c | 6 ++++++
6 files changed, 10 insertions(+), 59 deletions(-)
delete mode 100644 include/linux/platform_data/microread.h
^ permalink raw reply
* Re: [PATCH net-next 3/4] ldmvsw: Add ldmvsw.c driver code
From: aaron.young @ 2016-03-11 20:46 UTC (permalink / raw)
To: David Miller
Cc: netdev, sowmini.varadhan, alexandre.chartre, rashmi.narasimhan
In-Reply-To: <20160311.143022.2226126311284772576.davem@davemloft.net>
Thank you very much for the review. I'll make the requested changes
and resubmit the series...
-Aaron Young
On 03/11/16 11:30, David Miller wrote:
> From: Aaron Young <Aaron.Young@oracle.com>
> Date: Tue, 8 Mar 2016 07:02:35 -0800
>
>> +static struct vnet *vsw_get_vnet(struct mdesc_handle *hp,
>> + u64 port_node,
>> + u64 *handle)
>> +{
>> + struct vnet *vp;
>> + struct vnet *iter;
>> + const u64 *local_mac = NULL;
>> + const u64 *cfghandle = NULL;
>> + u64 a;
> Please order local variable declarations from longest to shortest line (reverse
> christmas tree).
>
>> +static int vsw_port_probe(struct vio_dev *vdev, const struct vio_device_id *id)
>> +{
>> + struct mdesc_handle *hp;
>> + struct vnet_port *port;
>> + unsigned long flags;
>> + struct vnet *vp;
>> + struct net_device *dev;
>> + const u64 *rmac;
>> + int len, i, err;
>> + const u64 *port_id;
>> + u64 handle;
> Likewise.
>
>> + err = register_netdev(dev);
>> + if (err) {
>> + pr_err("Cannot register net device, aborting\n");
>> + goto err_out_free_ldc;
>> + }
>> +
>> + netif_napi_add(dev, &port->napi, vnet_poll_common,
>> + NAPI_POLL_WEIGHT);
>> +
>> + INIT_LIST_HEAD(&port->list);
> You cannot register the netdevice so early. It must be registerred only after
> every single piece of software state is initialized and setup.
>
> At the very precise moment you invoke register_netdev() the device can
> be brought up and attempted to be used.
^ permalink raw reply
* Re: [RFC] net: ipv4 -- Introduce ifa limit per net
From: David Miller @ 2016-03-11 20:40 UTC (permalink / raw)
To: gorcunov
Cc: xiyou.wangcong, alexei.starovoitov, eric.dumazet, netdev, solar,
vvs, avagin, xemul, vdavydov, khorenko, pablo, netfilter-devel
In-Reply-To: <20160310224056.GF1989@uranus.lan>
From: Cyrill Gorcunov <gorcunov@gmail.com>
Date: Fri, 11 Mar 2016 01:40:56 +0300
> On Thu, Mar 10, 2016 at 05:36:30PM -0500, David Miller wrote:
>> >
>> > Works like a charm! So David, what are the next steps then?
>> > Mind to gather all your patches into one (maybe)?
>>
>> I'll re-review all of the changes tomorrow and also look into ipv6
>> masq, to see if it needs the same treatment, as well.
>>
>> Thanks for all of your help and testing so far.
>
> Thanks a lot, David!
Cyrill please retest this final patch and let me know if it still works
properly.
I looked at ipv6, and it's more complicated. The problem is that ipv6
doesn't mark the inet6dev object as dead in the NETDEV_DOWN case, in
fact it keeps the object around. It only releases it and marks it
dead in the NETDEV_UNREGISTER case.
We pay a very large price for having allowed the behavior of ipv6 and
ipv4 to diverge so greatly in these areas :-(
Nevertheless we should try to fix it somehow, maybe we can detect the
situation in another way for the ipv6 side.
====================
ipv4: Don't do expensive useless work during inetdev destroy.
When an inetdev is destroyed, every address assigned to the interface
is removed. And in this scenerio we do two pointless things which can
be very expensive if the number of assigned interfaces is large:
1) Address promotion. We are deleting all addresses, so there is no
point in doing this.
2) A full nf conntrack table purge for every address. We only need to
do this once, as is already caught by the existing
masq_dev_notifier so masq_inet_event() can skip this.
Reported-by: Cyrill Gorcunov <gorcunov@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index f6303b1..0212591 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -334,6 +334,9 @@ static void __inet_del_ifa(struct in_device *in_dev, struct in_ifaddr **ifap,
ASSERT_RTNL();
+ if (in_dev->dead)
+ goto no_promotions;
+
/* 1. Deleting primary ifaddr forces deletion all secondaries
* unless alias promotion is set
**/
@@ -380,6 +383,7 @@ static void __inet_del_ifa(struct in_device *in_dev, struct in_ifaddr **ifap,
fib_del_ifaddr(ifa, ifa1);
}
+no_promotions:
/* 2. Unlink it */
*ifap = ifa1->ifa_next;
diff --git a/net/ipv4/netfilter/nf_nat_masquerade_ipv4.c b/net/ipv4/netfilter/nf_nat_masquerade_ipv4.c
index c6eb421..ea91058 100644
--- a/net/ipv4/netfilter/nf_nat_masquerade_ipv4.c
+++ b/net/ipv4/netfilter/nf_nat_masquerade_ipv4.c
@@ -108,10 +108,18 @@ static int masq_inet_event(struct notifier_block *this,
unsigned long event,
void *ptr)
{
- struct net_device *dev = ((struct in_ifaddr *)ptr)->ifa_dev->dev;
+ struct in_device *idev = ((struct in_ifaddr *)ptr)->ifa_dev;
struct netdev_notifier_info info;
- netdev_notifier_info_init(&info, dev);
+ /* The masq_dev_notifier will catch the case of the device going
+ * down. So if the inetdev is dead and being destroyed we have
+ * no work to do. Otherwise this is an individual address removal
+ * and we have to perform the flush.
+ */
+ if (idev->dead)
+ return NOTIFY_DONE;
+
+ netdev_notifier_info_init(&info, idev->dev);
return masq_device_event(this, event, &info);
}
^ permalink raw reply related
* Re: Generic TSO (was Re: [net-next PATCH 0/2] GENEVE/VXLAN: Enable outer Tx checksum by default)
From: Edward Cree @ 2016-03-11 20:24 UTC (permalink / raw)
To: Tom Herbert; +Cc: Linux Kernel Network Developers
In-Reply-To: <CALx6S37P+gVA_O7vxBjGMxHpY69Cx2aL3E79ptvGkRW4L2x5Ug@mail.gmail.com>
On 11/03/16 20:16, Tom Herbert wrote:
> On Fri, Mar 11, 2016 at 11:59 AM, Edward Cree <ecree@solarflare.com> wrote:
>> On 11/03/16 19:57, Tom Herbert wrote:
>>> On Fri, Mar 11, 2016 at 11:20 AM, Edward Cree <ecree@solarflare.com> wrote:
>>>> Tom,
>>>>
>>>> Are you planning to / working on implementing this? If not, I might have a
>>>> crack at it; I've talked to our firmware guys and (provisionally) we think
>>>> we can support it in current sfc hardware.
>>>> Or were there any blocking problems raised in the thread? My understanding
>>>> of the IP ID issue was that it only matters for the inner frame, because
>>>> the rest aren't TCP (so hopefully no-one is doing SLHC on them). But I may
>>>> have missed something.
>>>>
>>> Right, then the interface would need to just include the offset of the
>>> IP ID. But doesn't this break using LCO with GSO though-- i.e. the
>>> outer checksum and inner checksum still need to be updated per packet
>>> so we need to tell device where outer checksum(s) is.
>> No, outer checksum shouldn't change: IP ID is protected by inner IP header
>> checksum, which device will edit. No?
> Right, the interface would probably still need offset to the IPv4 hdr?
Yes; I'm assuming the interface could just be "offset to inner IP header",
and the hardware knows well enough what IP and TCP headers look like that
it can figure out the rest (including skipping over options if e.g. ihl>5).
So, do you want to try and implement it or shall I?
-Ed
^ permalink raw reply
* Re: Generic TSO
From: David Miller @ 2016-03-11 20:22 UTC (permalink / raw)
To: ecree; +Cc: tom, netdev
In-Reply-To: <56E31A89.9020602@solarflare.com>
From: Edward Cree <ecree@solarflare.com>
Date: Fri, 11 Mar 2016 19:20:41 +0000
> Are you planning to / working on implementing this? If not, I might have a
> crack at it; I've talked to our firmware guys and (provisionally) we think
> we can support it in current sfc hardware.
> Or were there any blocking problems raised in the thread? My understanding
> of the IP ID issue was that it only matters for the inner frame, because
> the rest aren't TCP (so hopefully no-one is doing SLHC on them). But I may
> have missed something.
I've thought a lot more about the IP ID issue, and I am now starting
to learn towards allowing it to be set to zero for "DF" packets.
Considering what we gain in return, not working optimally with an
ancient SLIP header compression implementation is a small loss.
Any other opinions?
^ permalink raw reply
* Re: [PATCH net-next 0/4] qed: Management firmware updates
From: David Miller @ 2016-03-11 20:20 UTC (permalink / raw)
To: Yuval.Mintz; +Cc: netdev
In-Reply-To: <1457507786-6603-1-git-send-email-Yuval.Mintz@qlogic.com>
From: Yuval Mintz <Yuval.Mintz@qlogic.com>
Date: Wed, 9 Mar 2016 09:16:22 +0200
> This series contains several changes to driver interaction with the
> management fw.
> The biggest [& most significant] change here is a change in the locking
> scheme and re-definition of the 'critical section' when accessing shared
> resources toward the goal of interacting with the management firmware.
Series applied, thanks.
^ permalink raw reply
* Re: Generic TSO (was Re: [net-next PATCH 0/2] GENEVE/VXLAN: Enable outer Tx checksum by default)
From: Tom Herbert @ 2016-03-11 20:16 UTC (permalink / raw)
To: Edward Cree; +Cc: Linux Kernel Network Developers
In-Reply-To: <56E3239B.7080009@solarflare.com>
On Fri, Mar 11, 2016 at 11:59 AM, Edward Cree <ecree@solarflare.com> wrote:
> On 11/03/16 19:57, Tom Herbert wrote:
>> On Fri, Mar 11, 2016 at 11:20 AM, Edward Cree <ecree@solarflare.com> wrote:
>>> Tom,
>>>
>>> Are you planning to / working on implementing this? If not, I might have a
>>> crack at it; I've talked to our firmware guys and (provisionally) we think
>>> we can support it in current sfc hardware.
>>> Or were there any blocking problems raised in the thread? My understanding
>>> of the IP ID issue was that it only matters for the inner frame, because
>>> the rest aren't TCP (so hopefully no-one is doing SLHC on them). But I may
>>> have missed something.
>>>
>> Right, then the interface would need to just include the offset of the
>> IP ID. But doesn't this break using LCO with GSO though-- i.e. the
>> outer checksum and inner checksum still need to be updated per packet
>> so we need to tell device where outer checksum(s) is.
> No, outer checksum shouldn't change: IP ID is protected by inner IP header
> checksum, which device will edit. No?
Right, the interface would probably still need offset to the IPv4 hdr?
>
> -Ed
^ permalink raw reply
* Re: [PATCH net-next 0/4] BPF support for flow labels
From: David Miller @ 2016-03-11 20:14 UTC (permalink / raw)
To: daniel; +Cc: alexei.starovoitov, tgraf, netdev
In-Reply-To: <cover.1457488298.git.daniel@iogearbox.net>
From: Daniel Borkmann <daniel@iogearbox.net>
Date: Wed, 9 Mar 2016 03:00:01 +0100
> This set adds support for tunnel key flow labels for vxlan
> and geneve devices in collect meta data mode and eBPF support
> for managing these. For details please see individual patches.
Series applied, although I agree with the calls for more consolidation.
Thanks.
^ permalink raw reply
* Re: Generic TSO (was Re: [net-next PATCH 0/2] GENEVE/VXLAN: Enable outer Tx checksum by default)
From: Edward Cree @ 2016-03-11 19:59 UTC (permalink / raw)
To: Tom Herbert; +Cc: Linux Kernel Network Developers
In-Reply-To: <CALx6S37G5N-3GxXYf8uF9p8QNC+U_d6-Vf+8Jg6bFqEv_FpULg@mail.gmail.com>
On 11/03/16 19:57, Tom Herbert wrote:
> On Fri, Mar 11, 2016 at 11:20 AM, Edward Cree <ecree@solarflare.com> wrote:
>> Tom,
>>
>> Are you planning to / working on implementing this? If not, I might have a
>> crack at it; I've talked to our firmware guys and (provisionally) we think
>> we can support it in current sfc hardware.
>> Or were there any blocking problems raised in the thread? My understanding
>> of the IP ID issue was that it only matters for the inner frame, because
>> the rest aren't TCP (so hopefully no-one is doing SLHC on them). But I may
>> have missed something.
>>
> Right, then the interface would need to just include the offset of the
> IP ID. But doesn't this break using LCO with GSO though-- i.e. the
> outer checksum and inner checksum still need to be updated per packet
> so we need to tell device where outer checksum(s) is.
No, outer checksum shouldn't change: IP ID is protected by inner IP header
checksum, which device will edit. No?
-Ed
^ permalink raw reply
* Re: [PATCH] cisco: enic: Update logging macros and uses
From: David Miller @ 2016-03-11 20:08 UTC (permalink / raw)
To: joe; +Cc: benve, ssujith, _govind, neepatel, netdev, linux-kernel
In-Reply-To: <fd7e962bc72778fc1f2a9b21742eff2eb19d9bd8.1457474066.git.joe@perches.com>
From: Joe Perches <joe@perches.com>
Date: Tue, 8 Mar 2016 13:54:56 -0800
> Don't hide varibles used by the logging macros.
>
> Miscellanea:
>
> o Use the more common ##__VA_ARGS__ extension
> o Add missing newlines to formats
> o Realign arguments
>
> Signed-off-by: Joe Perches <joe@perches.com>
Applied, thanks Joe.
^ permalink raw reply
* Re: [PATCH net 0/3] bridge: ageing timer regression fix
From: David Miller @ 2016-03-11 20:04 UTC (permalink / raw)
To: stephen; +Cc: netdev, jiri
In-Reply-To: <1457470775-32224-1-git-send-email-stephen@networkplumber.org>
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Tue, 8 Mar 2016 12:59:32 -0800
> This fixes regression in how ageing timer is managed.
> Backing out the change required fixing switch drivers as well.
Series applied, thanks.
Jiri, since I applied this to net-next, I had to deal with the conflicts
created by your world splitup of the rocker driver. I wasn't sure, while
doing so, whether you'd like this new ageing_time member to remain in the
rocker struct, as in Stephen's patch, or to be placed in the ofdpa struct.
I choose the former, send me a patch if you want the latter.
I'll queue this up for -stable, as well.
^ permalink raw reply
* Re: [PATCH 2/2] isdn: i4l: move active-isdn drivers to staging
From: Tilman Schmidt @ 2016-03-11 20:04 UTC (permalink / raw)
To: isdn, Paul Bolle
Cc: devel, Arnd Bergmann, linux-doc, Greg Kroah-Hartman,
Jonathan Corbet, linux-kernel, netdev, Christoph Biedl,
David S. Miller, linux-arm-kernel
In-Reply-To: <56E1A39F.1010005@linux-pingi.de>
[-- Attachment #1.1.1: Type: text/plain, Size: 1297 bytes --]
Am 10.03.2016 um 17:41 schrieb isdn@linux-pingi.de:
> Am 10.03.2016 um 13:58 schrieb Paul Bolle:
>> On do, 2016-03-10 at 11:53 +0100, isdn@linux-pingi.de wrote:
>>> mISDN with CAPI support works just fine with pppd and pppdcapiplugin
>>> and the CAPI works for all mISDN HW.
>>
>> In the mainline tree the mISDN and CAPI stacks are effectively separate.
Correct.
> Since 2012 mISDN has a cAPI20 interface, pure in userspace.
To expand: The documented interface for CAPI 2.0 applications is the
shared library libcapi20.so. Originally that library just interfaced to
the kernel CAPI subsystem through /dev/capi20. Later it was extended to
support different access paths to ISDN devices:
- via /dev/capi20 and kernel CAPI as before
- over the network and a remote CAPI server running rcapid
- over the network to FRITZ!Box router via AVM's CAPI-over-TCP service
- last but not least, via the mISDNcapid daemon and mISDN
Of course this cuts off anything that doesn't pass through libcapi20.so,
including applications that (against the standard) access /dev/capi20
directly but also the capidrv.ko i4l compatibility shim.
--
Tilman Schmidt E-Mail: tilman@imap.cc
Bonn, Germany
Nous, on a des fleurs et des bougies pour nous protéger.
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
[-- Attachment #2: Type: text/plain, Size: 169 bytes --]
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply
* Re: [PATCH net-next 08/13] net/mlx5e: Add fragmented memory support for RX multi packet WQE
From: Eric Dumazet @ 2016-03-11 19:58 UTC (permalink / raw)
To: Saeed Mahameed
Cc: Saeed Mahameed, David S. Miller, Linux Netdev List, Or Gerlitz,
Eran Ben Elisha, Tal Alon, Tariq Toukan, Jesper Dangaard Brouer
In-Reply-To: <CALzJLG_XU-KMp8dMqcNOpu1OxZRnJro4LFQohK=FLX+5uJKXpQ@mail.gmail.com>
On ven., 2016-03-11 at 21:25 +0200, Saeed Mahameed wrote:
> >> -void mlx5e_handle_rx_cqe_mpwrq(struct mlx5e_rq *rq, struct mlx5_cqe64 *cqe)
> >> +static void mlx5e_add_skb_frag(struct sk_buff *skb, int len, struct page *page,
> >> + int page_offset)
> >> +{
> >> + int f = skb_shinfo(skb)->nr_frags++;
> >> + skb_frag_t *fr = &skb_shinfo(skb)->frags[f];
> >> +
> >> + skb->len += len;
> >> + skb->data_len += len;
> >> + get_page(page);
> >> + skb_frag_set_page(skb, f, page);
> >> + skb_frag_size_set(fr, len);
> >> + fr->page_offset = page_offset;
> >> + skb->truesize = SKB_TRUESIZE(skb->len);
> >> +}
> >
> > Really I am speechless.
> >
> > It is hard to believe how much effort some drivers authors spend trying
> > to fool linux stack and risk OOM a host under stress.
>
> Eric, you got it all wrong my friend, no one is trying to fool nobody here.
> I will explain it to you below.
>
> >
> > SKB_TRUESIZE() is absolutely not something a driver is allowed to use.
> >
> > Here you want instead :
> >
> > skb->truesize += PAGE_SIZE;
> >
> > Assuming you allocate and use an order-0 page per fragment. Fact that
> > you receive say 100 bytes datagram is irrelevant to truesize.
>
> Your assumption is wrong, we allocate as many pages as a WQE needs,
> and a WQE can describe/handle
> up to 1024 packets which share the same page/pages, so the skb should
> really have a true size of the strides
> of that page it used and not the WHOLE page as you think.
>
> you should already learn this from the previous patch.
>
> each WQE (Receive Work Queue Element) contains 1024 strides each of
> the size 128B,
> i.e, a packet of the size 128B or less will consume only one stride of
> that WQE page, next packets on that WQE
> will use the following strides in that same page.
>
> So in opposite of what you think this new scheme is better than our
> old one in terms of memory utilization.
> before, we wasted MTU size per SKB/Packet regardless of the real
> packet size, now each SKB will consume only
> as much as 128B strides it will need, no more no less.
>
> BTW there will be only 16 WQEs per ring :), so this new approach
> doesn't drastically consume more memory than the previous one.
> But it sure can handle more small packets bursts.
>
> >
> > truesize is the real memory usage of one skb. Not the minimal size of an
> > optimally allocated skb for a given payload.
>
> I totally agree with this, we should have reported skb->truesize +=
> (consumed strides)*(stride size).
> but again this is not as critical as you think, in the worst case
> skb->truesize will be off by 127B at most.
Ouch. really you are completely wrong.
If one skb has a fragment of a page, and sits in a queue for a long
time, it really uses a full page, because the remaining part of the page
is not reusable. Only kmalloc(128) can deal with that idea of allowing
other parts of the page being 'freed and reusable'
It is trivial for an attacker to make sure the host will consume one
page + sk_buff + skb->head = 4096 + 256 + 512, by specially sending out
of order packets on TCP flows.
It is very tempting to have special memory allocators you know, but you
have to understand attackers are smart. Smarter than us.
If now you are telling me you plan to allocate 131072 bytes pages (1024
strides of 128 bytes), then a smart attacker can actually bump skb
truesize to 128KB
Really your RX allocation schem is easily DOSable.
^ permalink raw reply
* Re: Generic TSO (was Re: [net-next PATCH 0/2] GENEVE/VXLAN: Enable outer Tx checksum by default)
From: Tom Herbert @ 2016-03-11 19:57 UTC (permalink / raw)
To: Edward Cree; +Cc: Linux Kernel Network Developers
In-Reply-To: <56E31A89.9020602@solarflare.com>
On Fri, Mar 11, 2016 at 11:20 AM, Edward Cree <ecree@solarflare.com> wrote:
> On 20/02/16 19:51, Tom Herbert wrote:
>> Right. To use LCO with TSO we would need to ensure that all packets
>> are the same size so that the UDP length field and thus checksum are
>> constant for all created segments. But this property this would also
>> make any payload lengths in headers constant for all packets so that
>> the only fields that need be set per generated packet would be the TCP
>> sequence number and checksum. This simplifying assumption could be
>> used to make a very protocol-generic GSO/TSO (up to the transport
>> header)!
>>
>> Conceptually, a device would just need to know the start of the
>> packet, the offset of the transport header, and the size of each
>> segment. Any bits from the start of the packet to the beginning of the
>> transport header are just copied to each segment, so any combination
>> of encapsulation/network protocols is supported as long as they are
>> constant for each segment (e.g. MPLS, NSH, etc. are on the horizon for
>> needing TSO support).
> Tom,
>
> Are you planning to / working on implementing this? If not, I might have a
> crack at it; I've talked to our firmware guys and (provisionally) we think
> we can support it in current sfc hardware.
> Or were there any blocking problems raised in the thread? My understanding
> of the IP ID issue was that it only matters for the inner frame, because
> the rest aren't TCP (so hopefully no-one is doing SLHC on them). But I may
> have missed something.
>
Right, then the interface would need to just include the offset of the
IP ID. But doesn't this break using LCO with GSO though-- i.e. the
outer checksum and inner checksum still need to be updated per packet
so we need to tell device where outer checksum(s) is.
Thanks,
Tom
> -Ed
^ permalink raw reply
* Re: Micrel Phy - Is there a way to configure the Phy not to do 802.3x flow control?
From: Florian Fainelli @ 2016-03-11 19:51 UTC (permalink / raw)
To: Murali Karicheri, johan, open list:TI NETCP ETHERNET DRIVER,
Kwok, WingMan, Andrew Lunn, opendmb
In-Reply-To: <56E30EE5.4050904@ti.com>
On 11/03/16 10:31, Murali Karicheri wrote:
> On 03/10/2016 02:38 PM, Murali Karicheri wrote:
>> On 03/10/2016 01:05 PM, Florian Fainelli wrote:
>>> On 10/03/16 08:48, Murali Karicheri wrote:
>>>> On 03/03/2016 07:16 PM, Florian Fainelli wrote:
>>>>> On 03/03/16 14:18, Murali Karicheri wrote:
>>>>>> Hi,
>>>>>>
>>>>>> We are using Micrel Phy in one of our board and wondering if we can force the
>>>>>> Phy to disable flow control at start. I have a 1G ethernet switch connected
>>>>>> to Phy and the phy always enable flow control. I would like to configure the
>>>>>> phy not to flow control. Is that possible and if yes, what should I do in the
>>>>>> my Ethernet driver to tell the Phy not to enable flow control?
>>>>>
>>>>> The PHY is not doing flow control per-se, your pseudo Ethernet MAC in
>>>>> the switch is doing, along with the link partner advertising support for
>>>>> it. You would want to make sure that your PHY device interface (provided
>>>>> that you are using the PHY library) is not starting with Pause
>>>>> advertised, but it could be supported.
>>>>
>>>> Understood that Phy is just advertise FC. The Micrel phy for 9031 advertise
>>>> by default FC supported. After negotiation, I see that Phylib provide the
>>>> link status with parameter pause = 1, asym_pause = 1. How do I tell the Phy not
>>>> to advertise?
>>>>
>>>> I call following sequence in the Ethernet driver.
>>>>
>>>> of_phy_connect(x,y,hndlr,a,z);
>>>
>>> Here you should be able to change phydev->advertising and
>>> phydev->supported to mask the ADVERTISED_Pause | ADVERTISED_AsymPause
>>> bits and have phy_start() restart with that which should disable pause
>>> and asym_pause as seen by your adjust_link handler.
>>>
>> Ok. Good point. I will try this. Thanks for your suggestion.
>>
> I had to make following changes to the phy_device.c to allow the phy device
> report maximum common flow control capability to Ethernet driver through
> handler. My driver code looks like this.
>
> slave->phy = of_phy_connect(gbe_intf->ndev,
> slave->phy_node,
> hndlr, 0,
> phy_mode);
> if (!slave->phy) {
> dev_err(priv->dev, "phy not found on slave %d\n",
> slave->slave_num);
> return -ENODEV;
> }
> dev_dbg(priv->dev, "phy found: id is: 0x%s\n",
> dev_name(&slave->phy->dev));
>
> slave->phy->supported &=
> ~(SUPPORTED_Pause | SUPPORTED_Asym_Pause);
> slave->phy->advertising = slave->phy->supported;
> phy_start(slave->phy);
> phy_read_status(slave->phy);
>
> And then in the phy_device.c, I did to get flow control off reported in
> handler for link status update.
>
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index d551df6..55412ad 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -1021,8 +1021,8 @@ int genphy_read_status(struct phy_device *phydev)
> phydev->duplex = DUPLEX_FULL;
>
> if (phydev->duplex == DUPLEX_FULL) {
> - phydev->pause = lpa & LPA_PAUSE_CAP ? 1 : 0;
> - phydev->asym_pause = lpa & LPA_PAUSE_ASYM ? 1 : 0;
> + phydev->pause = adv & lpa & LPA_PAUSE_CAP ? 1 : 0;
> + phydev->asym_pause = adv & lpa & LPA_PAUSE_ASYM ? 1 : 0;
What it means before your patch is that flow control is reported to the
PHY device if the link partner advertises that, with your patch applied,
it is reported only if the link partner and yourself advertise flow control.
You seem to be willing to have phydev->pause and phydev->asym_pause
reflect the resolved pause capability, as opposed to the link partner's
pause capability, which I am not convinced is correct here, because we
need to take into account the user-configured pause configuration as
well. Your adjust_link function should be the one deciding whether pause
frame advertising and enabling is appropriate based on: locally
configured pause settings (enabled, disabled, autoneg) and the link
partner's pause capability.
I do agree that the two fields are confusing and poorly documented, and
we should probably be consolidating the pause frame behavior in PHYLIB
as opposed to letting drivers deal with like e.g: gianfar, bcm63xx_enet,
tg3 etc.
>
> Could you explain, why the common maximum capability is not reported to the
> driver as per standard?? Or Am I understood it wrong?
I do not understand the question, what is "maximum capability" in that
context and what standard are you refering to?
--
Florian
^ permalink raw reply
* Re: [PATCH net] macvtap: always pass ethernet header in linear
From: David Miller @ 2016-03-11 19:45 UTC (permalink / raw)
To: willemdebruijn.kernel; +Cc: netdev, willemb
In-Reply-To: <1457468334-66777-1-git-send-email-willemdebruijn.kernel@gmail.com>
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date: Tue, 8 Mar 2016 15:18:54 -0500
> From: Willem de Bruijn <willemb@google.com>
>
> The stack expects link layer headers in the skb linear section.
> Macvtap can create skbs with llheader in frags in edge cases:
> when (IFF_VNET_HDR is off or vnet_hdr.hdr_len < ETH_HLEN) and
> prepad + len > PAGE_SIZE and vnet_hdr.flags has no or bad csum.
>
> Add checks to ensure linear is always at least ETH_HLEN.
> At this point, len is already ensured to be >= ETH_HLEN.
>
> For backwards compatiblity, rounds up short vnet_hdr.hdr_len.
> This differs from tap and packet, which return an error.
>
> Fixes b9fb9ee07e67 ("macvtap: add GSO/csum offload support")
> Signed-off-by: Willem de Bruijn <willemb@google.com>
Applied to net-next and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH net] ppp: ensure file->private_data can't be overridden
From: David Miller @ 2016-03-11 19:42 UTC (permalink / raw)
To: g.nault; +Cc: netdev, paulus, alan, arnd
In-Reply-To: <3f04de903b6be9e1c30427830c0a1568478e0c6c.1457462784.git.g.nault@alphalink.fr>
From: Guillaume Nault <g.nault@alphalink.fr>
Date: Tue, 8 Mar 2016 20:14:30 +0100
> Lock ppp_mutex and check that file->private_data is NULL before
> executing any action in ppp_unattached_ioctl().
> The test done by ppp_ioctl() can't be relied upon, because
> file->private_data may have been updated meanwhile. In which case
> ppp_unattached_ioctl() will override file->private_data and mess up
> reference counters or loose pointer to previously allocated PPP unit.
>
> In case the test fails, -ENOTTY is returned, just like if ppp_ioctl()
> had rejected the ioctl in the first place.
>
> Signed-off-by: Guillaume Nault <g.nault@alphalink.fr>
If this thing can disappear on us, then we need to make the entirety
of ppp_ioctl() run with the mutex held to fix this properly.
Otherwise ->private_data could go NULL on us meanwhile as well.
We should hold the mutex, to stabilize the value of ->private_data.
^ permalink raw reply
* Re: [PATCH] vmxnet3: avoid calling pskb_may_pull with interrupts disabled
From: Shrikrishna Khare @ 2016-03-11 19:41 UTC (permalink / raw)
To: Tetsuo Handa; +Cc: nhorman, davem, netdev, pv-drivers
In-Reply-To: <201603112222.CDG97029.OJFHQOtVOLFMFS@I-love.SAKURA.ne.jp>
On Fri, 11 Mar 2016, Tetsuo Handa wrote:
> Neil Horman wrote:
> > On Mon, Mar 07, 2016 at 03:16:14PM -0500, David Miller wrote:
> > > From: Neil Horman <nhorman@tuxdriver.com>
> > > Date: Fri, 4 Mar 2016 13:40:48 -0500
>
> This patch is calling spin_unlock_irqrestore() without spin_lock_irqsave().
>
> In file included from include/linux/seqlock.h:35:0,
> from include/linux/time.h:5,
> from include/linux/stat.h:18,
> from include/linux/module.h:10,
> from drivers/net/vmxnet3/vmxnet3_drv.c:27:
> drivers/net/vmxnet3/vmxnet3_drv.c: In function 'vmxnet3_xmit_frame':
> include/linux/spinlock.h:246:30: warning: 'flags' may be used uninitialized in this function [-Wmaybe-uninitialized]
> _raw_spin_unlock_irqrestore(lock, flags); \
> ^
> drivers/net/vmxnet3/vmxnet3_drv.c:977:16: note: 'flags' was declared here
> unsigned long flags;
> ^
>
> vmxnet3_tq_xmit(struct sk_buff *skb, struct vmxnet3_tx_queue *tq,
> struct vmxnet3_adapter *adapter, struct net_device *netdev)
> {
> (...snipped...)
> goto hdr_too_big;
> (...snipped...)
> spin_lock_irqsave(&tq->tx_lock, flags);
> (...snipped...)
> spin_unlock_irqrestore(&tq->tx_lock, flags);
> (...snipped...)
> return NETDEV_TX_OK;
> (...snipped...)
> hdr_too_big:
> tq->stats.drop_oversized_hdr++;
> unlock_drop_pkt:
> spin_unlock_irqrestore(&tq->tx_lock, flags);
> (...snipped...)
> return NETDEV_TX_OK;
> }
>
Thank you for reporting this. Will send out a fix later today.
^ permalink raw reply
* Re: [PATCH 1/3] net: thunderx: Cleanup PHY probing code.
From: Florian Fainelli @ 2016-03-11 19:37 UTC (permalink / raw)
To: Andrew Lunn, David Daney
Cc: David Daney, David S. Miller, netdev, linux-arm-kernel,
Robert Richter, Sunil Goutham, Kumar Gala, Ian Campbell,
Mark Rutland, Pawel Moll, Rob Herring, Radha Mohan Chintakuntla,
linux-kernel, David Daney
In-Reply-To: <20160311190627.GC19277@lunn.ch>
On 11/03/16 11:06, Andrew Lunn wrote:
>>> I don't see why it should wait around forever. I have boards with
>>> Marvell PHYs, yet if i don't build the Marvell driver, the Ethernet
>>> driver still loads, because the generic PHY driver is used instead.
>>> Why does this not work here?
>>
>> As I said before, there is no driver for the device, so
>> of_phy_find_device() will always return NULL.
>
> I'm not yet convinced this is true. I really do expect that the
> generic PHY driver will bind to it. It might then go horribly wrong,
> because it is not standard compliant, but that is a different issue.
I concur with Andrew here, unless the PHY is guaranteed to return
garbage when get_phy_id() is called, there is a good chance that the
Generic PHY driver will be bound to this PHY device, or this is not
happening for you for some reason?
>
> The generic driver should probably have a black list for such devices.
> This is a PHY issue, not an MDIO issue, and the problem should be
> solved in the PHY layer, not in one MDIO driver.
I considered the possibility once of disabling the generic PHY driver,
such that systems where the vendor-specific PHY driver is expected to be
used could utilize that. That does not play well with the fixed PHYs
using the generic PHY driver though, anyway, I am digressing.
>
> We should also consider what happens when somebody actually writes a
> driver for this PHY. Are you not going to use it?
>
> Before this patchset, you did not special case this compatible
> string. So at the very least, you need to split this into a separate
> patch, so the maintainers can ACK/NACK it, independent of the other
> change it is embedded in.
>
> Andrew
>
--
Florian
^ permalink raw reply
* Re: [PATCH 1/3] net: thunderx: Cleanup PHY probing code.
From: David Daney @ 2016-03-11 19:34 UTC (permalink / raw)
To: Andrew Lunn
Cc: David Daney, David S. Miller, netdev, linux-arm-kernel,
Florian Fainelli, Robert Richter, Sunil Goutham, Kumar Gala,
Ian Campbell, Mark Rutland, Pawel Moll, Rob Herring,
Radha Mohan Chintakuntla, linux-kernel, David Daney
In-Reply-To: <20160311190627.GC19277@lunn.ch>
On 03/11/2016 11:06 AM, Andrew Lunn wrote:
>>> I don't see why it should wait around forever. I have boards with
>>> Marvell PHYs, yet if i don't build the Marvell driver, the Ethernet
>>> driver still loads, because the generic PHY driver is used instead.
>>> Why does this not work here?
>>
>> As I said before, there is no driver for the device, so
>> of_phy_find_device() will always return NULL.
>
> I'm not yet convinced this is true.
Which part don't you believe? Is it:
- there is no driver for the device.
or
- for PHYs with no driver, of_phy_find_device() will return NULL
> I really do expect that the
> generic PHY driver will bind to it. It might then go horribly wrong,
> because it is not standard compliant, but that is a different issue.
>
At a higher level, the way we handle either of:
- Lack of a driver.
- "Horribly wrong" driver
is the same, we cannot use a PHY driver.
> The generic driver should probably have a black list for such devices.
> This is a PHY issue, not an MDIO issue, and the problem should be
> solved in the PHY layer, not in one MDIO driver.
This isn't an MDIO driver patch. This is more about handling a
defective device tree in the only driver (a non-MDIO driver) that will
ever see such a device tree node.
>
> We should also consider what happens when somebody actually writes a
> driver for this PHY. Are you not going to use it?
Easy to answer: We remove the "&& !of_device_is_compatible(phy_np,
"cortina,cs4223-slice")" clause from this driver.
>
> Before this patchset, you did not special case this compatible
> string. So at the very least, you need to split this into a separate
> patch, so the maintainers can ACK/NACK it, independent of the other
> change it is embedded in.
>
I can, and will, do that.
Thanks,
David Daney
^ permalink raw reply
* Re: [PATCH net-next 3/4] ldmvsw: Add ldmvsw.c driver code
From: David Miller @ 2016-03-11 19:30 UTC (permalink / raw)
To: Aaron.Young
Cc: netdev, sowmini.varadhan, alexandre.chartre, rashmi.narasimhan
In-Reply-To: <f3e221f516c1514b7d60b8e8970130b4a31d1d59.1457401532.git.Aaron.Young@oracle.com>
From: Aaron Young <Aaron.Young@oracle.com>
Date: Tue, 8 Mar 2016 07:02:35 -0800
> +static struct vnet *vsw_get_vnet(struct mdesc_handle *hp,
> + u64 port_node,
> + u64 *handle)
> +{
> + struct vnet *vp;
> + struct vnet *iter;
> + const u64 *local_mac = NULL;
> + const u64 *cfghandle = NULL;
> + u64 a;
Please order local variable declarations from longest to shortest line (reverse
christmas tree).
> +static int vsw_port_probe(struct vio_dev *vdev, const struct vio_device_id *id)
> +{
> + struct mdesc_handle *hp;
> + struct vnet_port *port;
> + unsigned long flags;
> + struct vnet *vp;
> + struct net_device *dev;
> + const u64 *rmac;
> + int len, i, err;
> + const u64 *port_id;
> + u64 handle;
Likewise.
> + err = register_netdev(dev);
> + if (err) {
> + pr_err("Cannot register net device, aborting\n");
> + goto err_out_free_ldc;
> + }
> +
> + netif_napi_add(dev, &port->napi, vnet_poll_common,
> + NAPI_POLL_WEIGHT);
> +
> + INIT_LIST_HEAD(&port->list);
You cannot register the netdevice so early. It must be registerred only after
every single piece of software state is initialized and setup.
At the very precise moment you invoke register_netdev() the device can
be brought up and attempted to be used.
^ permalink raw reply
* Re: [PATCH net-next 04/13] net/mlx5e: Use only close NUMA node for default RSS
From: Saeed Mahameed @ 2016-03-11 19:29 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Saeed Mahameed, David S. Miller, Linux Netdev List, Or Gerlitz,
Eran Ben Elisha, Tal Alon, Tariq Toukan, Jesper Dangaard Brouer
In-Reply-To: <56E2D177.3090403@cogentembedded.com>
On Fri, Mar 11, 2016 at 4:08 PM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> Hello.
>
> On 3/11/2016 4:39 PM, Saeed Mahameed wrote:
>
>> From: Tariq Toukan <tariqt@mellanox.com>
>>
>> Distribute default RSS table uniformely over the rings of the
>
>
> Uniformly.
Indeed :), will fix this
Thank you.
^ permalink raw reply
* Re: [PATCH net-next 2/4] ldmvsw: Make sunvnet_common compatible with ldmvsw
From: David Miller @ 2016-03-11 19:27 UTC (permalink / raw)
To: Aaron.Young
Cc: netdev, sowmini.varadhan, alexandre.chartre, rashmi.narasimhan
In-Reply-To: <85723889cbcaab6d1f72581f5d34b07c7dc67f6f.1457401532.git.Aaron.Young@oracle.com>
From: Aaron Young <Aaron.Young@oracle.com>
Date: Tue, 8 Mar 2016 07:02:34 -0800
> @@ -719,12 +720,13 @@ static void maybe_tx_wakeup(struct vnet_port *port)
> __netif_tx_unlock(txq);
> }
>
> -static inline bool port_is_up(struct vnet_port *vnet)
> +inline bool port_is_up_common(struct vnet_port *vnet)
All of these things in foo.c files shouldn't have an inline attribute.
> {
> struct vio_driver_state *vio = &vnet->vio;
>
> return !!(vio->hs_state & VIO_HS_COMPLETE);
> }
> +EXPORT_SYMBOL(port_is_up_common);
Especially if they are also exported.
Need to use EXPORT_SYMBOL_GPL() here as well, but I seriously think you need to rename
this function in order to not pollute the global namespace of exported kernel symbols.
You need to add a suitable prefix such as sunvnet_*() or whatever.
^ permalink raw reply
* Re: [PATCH net-next 1/4] ldmvsw: Split sunvnet driver into common code
From: David Miller @ 2016-03-11 19:26 UTC (permalink / raw)
To: Aaron.Young
Cc: netdev, sowmini.varadhan, alexandre.chartre, rashmi.narasimhan
In-Reply-To: <25112e068360c00b9737797df40a3a6f25a3cd22.1457401532.git.Aaron.Young@oracle.com>
From: Aaron Young <Aaron.Young@oracle.com>
Date: Tue, 8 Mar 2016 07:02:33 -0800
> +EXPORT_SYMBOL(vnet_send_attr_common);
All of these need to be EXPORT_SYMBOL_GPL()
^ permalink raw reply
* Re: [PATCH net-next 08/13] net/mlx5e: Add fragmented memory support for RX multi packet WQE
From: Saeed Mahameed @ 2016-03-11 19:25 UTC (permalink / raw)
To: Eric Dumazet
Cc: Saeed Mahameed, David S. Miller, Linux Netdev List, Or Gerlitz,
Eran Ben Elisha, Tal Alon, Tariq Toukan, Jesper Dangaard Brouer
In-Reply-To: <1457706771.2663.37.camel@edumazet-ThinkPad-T530>
>> -void mlx5e_handle_rx_cqe_mpwrq(struct mlx5e_rq *rq, struct mlx5_cqe64 *cqe)
>> +static void mlx5e_add_skb_frag(struct sk_buff *skb, int len, struct page *page,
>> + int page_offset)
>> +{
>> + int f = skb_shinfo(skb)->nr_frags++;
>> + skb_frag_t *fr = &skb_shinfo(skb)->frags[f];
>> +
>> + skb->len += len;
>> + skb->data_len += len;
>> + get_page(page);
>> + skb_frag_set_page(skb, f, page);
>> + skb_frag_size_set(fr, len);
>> + fr->page_offset = page_offset;
>> + skb->truesize = SKB_TRUESIZE(skb->len);
>> +}
>
> Really I am speechless.
>
> It is hard to believe how much effort some drivers authors spend trying
> to fool linux stack and risk OOM a host under stress.
Eric, you got it all wrong my friend, no one is trying to fool nobody here.
I will explain it to you below.
>
> SKB_TRUESIZE() is absolutely not something a driver is allowed to use.
>
> Here you want instead :
>
> skb->truesize += PAGE_SIZE;
>
> Assuming you allocate and use an order-0 page per fragment. Fact that
> you receive say 100 bytes datagram is irrelevant to truesize.
Your assumption is wrong, we allocate as many pages as a WQE needs,
and a WQE can describe/handle
up to 1024 packets which share the same page/pages, so the skb should
really have a true size of the strides
of that page it used and not the WHOLE page as you think.
you should already learn this from the previous patch.
each WQE (Receive Work Queue Element) contains 1024 strides each of
the size 128B,
i.e, a packet of the size 128B or less will consume only one stride of
that WQE page, next packets on that WQE
will use the following strides in that same page.
So in opposite of what you think this new scheme is better than our
old one in terms of memory utilization.
before, we wasted MTU size per SKB/Packet regardless of the real
packet size, now each SKB will consume only
as much as 128B strides it will need, no more no less.
BTW there will be only 16 WQEs per ring :), so this new approach
doesn't drastically consume more memory than the previous one.
But it sure can handle more small packets bursts.
>
> truesize is the real memory usage of one skb. Not the minimal size of an
> optimally allocated skb for a given payload.
I totally agree with this, we should have reported skb->truesize +=
(consumed strides)*(stride size).
but again this is not as critical as you think, in the worst case
skb->truesize will be off by 127B at most.
I will discuss this with Tariq and fix it.
>
>
> Better RX speed should not be done at the risk of system stability.
The whole idea of this patch is not improving RX speed ! No ! not at
all ! it just improves the driver resiliency
when the system is under stress on the expense of performance!
So I really think we should get a "thumbs up" from you.
>
> Now if for some reason you need to increase max TCP RWIN, that would be
> a TCP stack change, not some obscure lie in a driver trying to be faster
> than competitors.
No we are not trying to max TCP RWIN in here, Sorry you think of it
this way, I hope my explanation above changes your mind.
Thanks,
Saeed
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox