* [PATCH 0/2] ixgbe updates
From: John Fastabend @ 2017-04-24 1:30 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev
Hi Jeff, Here are two patch updates for your tree. I merged in Alex's
build fix and Jakub's comments. Lightly tested xdp1 and xdp2.
---
John Fastabend (2):
ixgbe: add XDP support for pass and drop actions
ixgbe: add support for XDP_TX action
0 files changed
^ permalink raw reply
* Re: [PATCH net-next V3 2/2] rtnl: Add support for netdev event attribute to link messages
From: David Ahern @ 2017-04-24 1:07 UTC (permalink / raw)
To: Vladislav Yasevich, netdev; +Cc: Vladislav Yasevich, roopa, Jiri Pirko
In-Reply-To: <1492795881-11914-3-git-send-email-vyasevic@redhat.com>
On 4/21/17 11:31 AM, Vladislav Yasevich wrote:
> @@ -1276,9 +1277,40 @@ static int rtnl_xdp_fill(struct sk_buff *skb, struct net_device *dev)
> return err;
> }
>
> +static int rtnl_fill_link_event(struct sk_buff *skb, unsigned long event)
> +{
> + u32 rtnl_event;
> +
> + switch (event) {
> + case NETDEV_REBOOT:
> + rtnl_event = IFLA_EVENT_REBOOT;
> + break;
> + case NETDEV_FEAT_CHANGE:
> + rtnl_event = IFLA_EVENT_FEAT_CHANGE;
> + break;
> + case NETDEV_BONDING_FAILOVER:
> + rtnl_event = IFLA_EVENT_BONDING_FAILOVER;
> + break;
> + case NETDEV_NOTIFY_PEERS:
> + rtnl_event = IFLA_EVENT_NOTIFY_PEERS;
> + break;
> + case NETDEV_RESEND_IGMP:
> + rtnl_event = IFLA_EVENT_RESEND_IGMP;
> + break;
> + case NETDEV_CHANGEINFODATA:
> + rtnl_event = IFLA_EVENT_CHANGE_INFO_DATA;
> + break;
> + default:
> + return 0;
> + }
> +
> + return nla_put_u32(skb, IFLA_EVENT, rtnl_event);
> +}
> +
I still have doubts about encoding kernel events into a uapi.
For example, NETDEV_CHANGEINFODATA is only for bonds though nothing
about the name suggests it is a bonding notification. This one was added
specifically to notify userspace (d4261e5650004), yet seems to happen
only during a changelink and that already generates a RTM_NEWLINK
message via do_setlink. Since the rtnetlink_event message does not
contain anything "NETDEV_CHANGEINFODATA" related what purpose does it
really serve besides duplicating netlink messages to userspace.
The REBOOT, IGMP, FEAT_CHANGE and BONDING_FAILOVER seem to be unique
messages (code analysis only) which I get for notifying userspace.
NETDEV_NOTIFY_PEERS is not so clear in how often it duplicates other
messages.
^ permalink raw reply
* [PATCH 1/1] rndis_wlan: add return value validation
From: Pan Bian @ 2017-04-24 0:40 UTC (permalink / raw)
To: Jussi Kivilinna, Kalle Valo, linux-wireless, netdev
Cc: linux-kernel, Pan Bian
From: Pan Bian <bianpan2016@163.com>
Function create_singlethread_workqueue() will return a NULL pointer if
there is no enough memory, and its return value should be validated
before using. However, in function rndis_wlan_bind(), its return value
is not checked. This may cause NULL dereference bugs. This patch fixes
it.
Signed-off-by: Pan Bian <bianpan2016@163.com>
---
drivers/net/wireless/rndis_wlan.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/wireless/rndis_wlan.c b/drivers/net/wireless/rndis_wlan.c
index 785334f..92a1bde 100644
--- a/drivers/net/wireless/rndis_wlan.c
+++ b/drivers/net/wireless/rndis_wlan.c
@@ -3427,6 +3427,10 @@ static int rndis_wlan_bind(struct usbnet *usbdev, struct usb_interface *intf)
/* because rndis_command() sleeps we need to use workqueue */
priv->workqueue = create_singlethread_workqueue("rndis_wlan");
+ if (!priv->workqueue) {
+ wiphy_free(wiphy);
+ return -ENOMEM;
+ }
INIT_WORK(&priv->work, rndis_wlan_worker);
INIT_DELAYED_WORK(&priv->dev_poller_work, rndis_device_poller);
INIT_DELAYED_WORK(&priv->scan_work, rndis_get_scan_results);
--
1.9.1
^ permalink raw reply related
* 10985 netdev
From: kelley @ 2017-04-24 0:24 UTC (permalink / raw)
To: netdev
[-- Attachment #1: 024976477871603.zip --]
[-- Type: application/zip, Size: 1559 bytes --]
^ permalink raw reply
* RE:drivers:net:ethernet:adi:bfin_mac: Use FIELD_SIZEOF defined kernel macro
From: Karim Eshapa @ 2017-04-24 0:23 UTC (permalink / raw)
To: davem
Cc: tremyfr, a, fw, clabbe.montjoie, edumazet, jarod,
adi-buildroot-devel, netdev, linux-kernel, Karim Eshapa
In-Reply-To: <1492970524-12607-1-git-send-email-karim.eshapa@gmail.com>
On Sun, 23 Apr 2017 22:56:38 +0200, Geert Uytterhoeven:
>IMHO this makes the code less safe and less future-proof.
>What if the type of info is ever changed?
>There's no safety check to validate that the FIELD_SIZEOF() operates on the
>same data as the strlcpy() destination.
Really make sense :)
>As the kbuild test robot already told you, this doesn't compile.
>Please try to (at least) compile the code before sending patches.
>Thanks!
So sorry for that I built quickly using the wrong compiler and the wrong
target machine with my old .config that's horrible :)
Thanks a lot,
Karim
^ permalink raw reply
* Re: [PATCH v4 net-next] mdio_bus: Issue GPIO RESET to PHYs.
From: Andrew Lunn @ 2017-04-23 23:35 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: Roger Quadros, davem, Florian Fainelli, tony, nsekhar, jsarha,
netdev, linux-omap, linux-kernel
In-Reply-To: <1c36aab1-3411-f76d-67f7-e3f6d9e9fda5@metafoo.de>
On Fri, Apr 21, 2017 at 03:31:09PM +0200, Lars-Peter Clausen wrote:
> On 04/21/2017 03:15 PM, Roger Quadros wrote:
> > diff --git a/Documentation/devicetree/bindings/net/mdio.txt b/Documentation/devicetree/bindings/net/mdio.txt
> > new file mode 100644
> > index 0000000..4ffbbac
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/mdio.txt
> > @@ -0,0 +1,33 @@
> > +Common MDIO bus properties.
> > +
> > +These are generic properties that can apply to any MDIO bus.
> > +
> > +Optional properties:
> > +- reset-gpios: List of one or more GPIOs that control the RESET lines
> > + of the PHYs on that MDIO bus.
> > +- reset-delay-us: RESET pulse width in microseconds as per PHY datasheet.
> > +
> > +A list of child nodes, one per device on the bus is expected. These
> > +should follow the generic phy.txt, or a device specific binding document.
> > +
> > +Example :
> > +This example shows these optional properties, plus other properties
> > +required for the TI Davinci MDIO driver.
> > +
> > + davinci_mdio: ethernet@0x5c030000 {
> > + compatible = "ti,davinci_mdio";
> > + reg = <0x5c030000 0x1000>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + reset-gpios = <&gpio2 5 GPIO_ACTIVE_LOW>;
> > + reset-delay-us = <2>; /* PHY datasheet states 1us min */
>
> If this is the reset line of the PHY shouldn't it be a property of the PHY
> node rather than of the MDIO controller node (which might have a reset on
> its own)?
> > +
> > + ethphy0: ethernet-phy@1 {
> > + reg = <1>;
> > + };
> > +
> > + ethphy1: ethernet-phy@3 {
> > + reg = <3>;
> > + };
Hi Lars-Peter
We discussed this when the first proposal was made. There are two
cases, to consider.
1) Here, one GPIO line resets all PHYs on the same MDIO bus. In this
example, two PHYs.
2) There is one GPIO line per PHY. That is a separate case, and as you
say, the reset line should probably be considered a PHY property, not
an MDIO property. However, it can be messy, since in order to probe
the MDIO bus, you probably need to take the PHY out of reset.
Anyway, this patch addresses the first case, so should be accepted. If
anybody wants to address the second case, they are free to do so.
Andrew
^ permalink raw reply
* Re: [PATCH RFC] ptr_ring: add ptr_ring_unconsume
From: Michael S. Tsirkin @ 2017-04-23 23:28 UTC (permalink / raw)
To: Jason Wang; +Cc: linux-kernel, netdev
In-Reply-To: <0defb746-3d4b-14b3-1ad7-82842048ebba@redhat.com>
On Tue, Apr 18, 2017 at 11:07:42AM +0800, Jason Wang wrote:
>
>
> On 2017年04月17日 07:19, Michael S. Tsirkin wrote:
> > Applications that consume a batch of entries in one go
> > can benefit from ability to return some of them back
> > into the ring.
> >
> > Add an API for that - assuming there's space. If there's no space
> > naturally we can't do this and have to drop entries, but this implies
> > ring is full so we'd likely drop some anyway.
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >
> > Jason, in my mind the biggest issue with your batching patchset is the
> > backet drops on disconnect. This API will help avoid that in the common
> > case.
>
> Ok, I will rebase the series on top of this. (Though I don't think we care
> the packet loss).
E.g. I care - I often start sending packets to VM before it's
fully booted. Several vhost resets might follow.
> >
> > I would still prefer that we understand what's going on,
>
> I try to reply in another thread, does it make sense?
>
> > and I would
> > like to know what's the smallest batch size that's still helpful,
>
> Yes, I've replied in another thread, the result is:
>
>
> no batching 1.88Mpps
> RX_BATCH=1 1.93Mpps
> RX_BATCH=4 2.11Mpps
> RX_BATCH=16 2.14Mpps
> RX_BATCH=64 2.25Mpps
> RX_BATCH=256 2.18Mpps
Essentially 4 is enough, other stuf looks more like noise
to me. What about 2?
> > but
> > I'm not going to block the patch on these grounds assuming packet drops
> > are fixed.
>
> Thanks a lot.
>
> >
> > Lightly tested - this is on top of consumer batching patches.
> >
> > Thanks!
> >
> > include/linux/ptr_ring.h | 57 ++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 57 insertions(+)
> >
> > diff --git a/include/linux/ptr_ring.h b/include/linux/ptr_ring.h
> > index 783e7f5..5fbeab4 100644
> > --- a/include/linux/ptr_ring.h
> > +++ b/include/linux/ptr_ring.h
> > @@ -457,6 +457,63 @@ static inline int ptr_ring_init(struct ptr_ring *r, int size, gfp_t gfp)
> > return 0;
> > }
> > +/*
> > + * Return entries into ring. Destroy entries that don't fit.
> > + *
> > + * Note: this is expected to be a rare slow path operation.
> > + *
> > + * Note: producer lock is nested within consumer lock, so if you
> > + * resize you must make sure all uses nest correctly.
> > + * In particular if you consume ring in interrupt or BH context, you must
> > + * disable interrupts/BH when doing so.
> > + */
> > +static inline void ptr_ring_unconsume(struct ptr_ring *r, void **batch, int n,
> > + void (*destroy)(void *))
> > +{
> > + unsigned long flags;
> > + int head;
> > +
> > + spin_lock_irqsave(&(r)->consumer_lock, flags);
> > + spin_lock(&(r)->producer_lock);
> > +
> > + if (!r->size)
> > + goto done;
> > +
> > + /*
> > + * Clean out buffered entries (for simplicity). This way following code
> > + * can test entries for NULL and if not assume they are valid.
> > + */
> > + head = r->consumer_head - 1;
> > + while (likely(head >= r->consumer_tail))
> > + r->queue[head--] = NULL;
> > + r->consumer_tail = r->consumer_head;
> > +
> > + /*
> > + * Go over entries in batch, start moving head back and copy entries.
> > + * Stop when we run into previously unconsumed entries.
> > + */
> > + while (n--) {
> > + head = r->consumer_head - 1;
> > + if (head < 0)
> > + head = r->size - 1;
> > + if (r->queue[head]) {
> > + /* This batch entry will have to be destroyed. */
> > + ++n;
> > + goto done;
> > + }
> > + r->queue[head] = batch[n];
> > + r->consumer_tail = r->consumer_head = head;
> > + }
> > +
> > +done:
> > + /* Destroy all entries left in the batch. */
> > + while (n--) {
> > + destroy(batch[n]);
> > + }
> > + spin_unlock(&(r)->producer_lock);
> > + spin_unlock_irqrestore(&(r)->consumer_lock, flags);
> > +}
> > +
> > static inline void **__ptr_ring_swap_queue(struct ptr_ring *r, void **queue,
> > int size, gfp_t gfp,
> > void (*destroy)(void *))
^ permalink raw reply
* I find one aspect of the ip link show command confusing and I'd like you to fix it, please
From: Jeff Silverman @ 2017-04-23 22:36 UTC (permalink / raw)
To: netdev
People,
When my NIC is up, but not connected, I see:
root@jeff-desktop:~# ip link show enp3s0
2: enp3s0: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc mq state
DOWN mode DEFAULT group default qlen 1000
link/ether 00:10:18:cc:9c:77 brd ff:ff:ff:ff:ff:ff
root@jeff-desktop:~# ip link show enp3s0
NO-CARRIER makes sense to me - if the wire is unplugged, then the NIC
isn't seeing the humm at the beginning of each packet. That's clear.
Note that even though my link isn't plugged in, ip still notes that
it is up. That's great.
But if I down my NIC, there is no indication that it is DOWN other
than you can't see the UP flag. If somebody was new to linux, they
would not see what's not there.
2: enp3s0: <BROADCAST,MULTICAST> mtu 1500 qdisc mq state DOWN mode
DEFAULT group default qlen 1000
link/ether 00:10:18:cc:9c:77 brd ff:ff:ff:ff:ff:ff
What I would like you to do is modify the ip command so that if the
NIC has been downed by something, then it explicitly says DOWN.
What would be really nice would be if you enumerated all of the flags
that an interface can have, and note if the flag is set or cleared.
But that's more than what I want with this message.
Many thanks,
Jeff
--
Jeff Silverman, linux sysadmin
nine two four twentieth avenue east
Seattle, WA, nine eight one one two -3507
(253) 459-2318
jeffsilverm@gmail.c0m (note the zero!)
http://www.commercialventvac.com
See my portfolio of writings and talks
^ permalink raw reply
* Re: [PATCH 1/1] drivers:net:ethernet:adi:bfin_mac: Use FIELD_SIZEOF defined kernel macro
From: Geert Uytterhoeven @ 2017-04-23 20:56 UTC (permalink / raw)
To: Karim Eshapa
Cc: David S. Miller, Philippe Reynes, Antonio Quartulli,
Florian Westphal, LABBE Corentin, Eric Dumazet, Jarod Wilson,
adi-buildroot-devel@lists.sourceforge.net, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <1492970524-12607-1-git-send-email-karim.eshapa@gmail.com>
Hi Karim,
On Sun, Apr 23, 2017 at 8:02 PM, Karim Eshapa <karim.eshapa@gmail.com> wrote:
> Use FIELD_SIZEOF defined kernel macro kernel.h
>
> Signed-off-by: Karim Eshapa <karim.eshapa@gmail.com>
> ---
> drivers/net/ethernet/adi/bfin_mac.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/adi/bfin_mac.c b/drivers/net/ethernet/adi/bfin_mac.c
> index a9ac58c..60346e0 100644
> --- a/drivers/net/ethernet/adi/bfin_mac.c
> +++ b/drivers/net/ethernet/adi/bfin_mac.c
> @@ -452,10 +452,14 @@ static irqreturn_t bfin_mac_wake_interrupt(int irq, void *dev_id)
> static void bfin_mac_ethtool_getdrvinfo(struct net_device *dev,
> struct ethtool_drvinfo *info)
> {
> - strlcpy(info->driver, KBUILD_MODNAME, sizeof(info->driver));
> - strlcpy(info->version, DRV_VERSION, sizeof(info->version));
> - strlcpy(info->fw_version, "N/A", sizeof(info->fw_version));
> - strlcpy(info->bus_info, dev_name(&dev->dev), sizeof(info->bus_info));
> + strlcpy(info->driver, KBUILD_MODNAME, FIELD_SIZEOF(
> + struct ethtool_drvinfo, driver));
IMHO this makes the code less safe and less future-proof.
What if the type of info is ever changed?
There's no safety check to validate that the FIELD_SIZEOF() operates on the
same data as the strlcpy() destination.
> + strlcpy(info->version, DRV_VERSION, FIELD_SIZEOF(
> + struct ethtool_drvinfo, version));
> + strlcpy(info->fw_version, "N/A", FIELD_SIZEOF(
> + struct ethtool_drvinfo, fw_version));
> + strlcpy(info->bus_info, dev_name(&dev->dev), FIELD_SIZEOF(
> + struct ethtool_drvinfo, bus_info));
> }
>
> static void bfin_mac_ethtool_getwol(struct net_device *dev,
> @@ -785,7 +789,7 @@ static int bfin_mac_hwtstamp_get(struct net_device *netdev,
> struct bfin_mac_local *lp = netdev_priv(netdev);
>
> return copy_to_user(ifr->ifr_data, &lp->stamp_cfg,
> - sizeof(lp->stamp_cfg)) ?
> + FILD_SIZEOF(struct bfin_mac_local, stamp_cfg)) ?
As the kbuild test robot already told you, this doesn't compile.
Please try to (at least) compile the code before sending patches.
Thanks!
Gr{oetje,eeting}s,
Geert
^ permalink raw reply
* Re: [PATCH 1/1] openvswitch: check return value of nla_nest_start
From: Pravin Shelar @ 2017-04-23 20:22 UTC (permalink / raw)
To: Pan Bian
Cc: ovs dev, Linux Kernel Network Developers, David S. Miller,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1492929782-1112-1-git-send-email-bianpan2016-9Onoh4P/yGk@public.gmane.org>
On Sat, Apr 22, 2017 at 11:43 PM, Pan Bian <bianpan2016-9Onoh4P/yGk@public.gmane.org> wrote:
> Function nla_nest_start() will return a NULL pointer on error, and its
> return value should be validated before it is used. However, in function
> queue_userspace_packet(), its return value is ignored. This may result
> in NULL dereference when calling nla_nest_end(). This patch fixes the
> bug.
>
> Signed-off-by: Pan Bian <bianpan2016-9Onoh4P/yGk@public.gmane.org>
> ---
> net/openvswitch/datapath.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
> index 9c62b63..34c0fbd 100644
> --- a/net/openvswitch/datapath.c
> +++ b/net/openvswitch/datapath.c
> @@ -489,7 +489,8 @@ static int queue_userspace_packet(struct datapath *dp, struct sk_buff *skb,
> err = ovs_nla_put_tunnel_info(user_skb,
> upcall_info->egress_tun_info);
> BUG_ON(err);
> - nla_nest_end(user_skb, nla);
> + if (nla)
> + nla_nest_end(user_skb, nla);
There is enough memory allocated for the netlink message accounting
for all attributes beforehand. So it is not required to check retuned
'nla' after each attributes addition to the msg.
> }
>
> if (upcall_info->actions_len) {
> @@ -497,7 +498,7 @@ static int queue_userspace_packet(struct datapath *dp, struct sk_buff *skb,
> err = ovs_nla_put_actions(upcall_info->actions,
> upcall_info->actions_len,
> user_skb);
> - if (!err)
> + if (!err && nla)
> nla_nest_end(user_skb, nla);
> else
> nla_nest_cancel(user_skb, nla);
> --
> 1.9.1
>
>
> _______________________________________________
> dev mailing list
> dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
^ permalink raw reply
* Re: [PATCH 1/1] drivers:net:ethernet:adi:bfin_mac: Use FIELD_SIZEOF defined kernel macro
From: kbuild test robot @ 2017-04-23 20:14 UTC (permalink / raw)
To: Karim Eshapa
Cc: kbuild-all, davem, tremyfr, a, fw, clabbe.montjoie, edumazet,
jarod, adi-buildroot-devel, netdev, linux-kernel, Karim Eshapa
In-Reply-To: <1492970524-12607-1-git-send-email-karim.eshapa@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2043 bytes --]
Hi Karim,
[auto build test ERROR on net-next/master]
[also build test ERROR on v4.11-rc7 next-20170421]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Karim-Eshapa/drivers-net-ethernet-adi-bfin_mac-Use-FIELD_SIZEOF-defined-kernel-macro/20170424-022248
config: blackfin-BF518F-EZBRD_defconfig (attached as .config)
compiler: bfin-uclinux-gcc (GCC) 6.2.0
reproduce:
wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=blackfin
All error/warnings (new ones prefixed by >>):
drivers/net/ethernet/adi/bfin_mac.c: In function 'bfin_mac_hwtstamp_get':
>> drivers/net/ethernet/adi/bfin_mac.c:792:8: error: implicit declaration of function 'FILD_SIZEOF' [-Werror=implicit-function-declaration]
FILD_SIZEOF(struct bfin_mac_local, stamp_cfg)) ?
^~~~~~~~~~~
>> drivers/net/ethernet/adi/bfin_mac.c:792:20: error: expected expression before 'struct'
FILD_SIZEOF(struct bfin_mac_local, stamp_cfg)) ?
^~~~~~
>> drivers/net/ethernet/adi/bfin_mac.c:794:1: warning: control reaches end of non-void function [-Wreturn-type]
}
^
cc1: some warnings being treated as errors
vim +/FILD_SIZEOF +792 drivers/net/ethernet/adi/bfin_mac.c
786 static int bfin_mac_hwtstamp_get(struct net_device *netdev,
787 struct ifreq *ifr)
788 {
789 struct bfin_mac_local *lp = netdev_priv(netdev);
790
791 return copy_to_user(ifr->ifr_data, &lp->stamp_cfg,
> 792 FILD_SIZEOF(struct bfin_mac_local, stamp_cfg)) ?
793 -EFAULT : 0;
> 794 }
795
796 static void bfin_tx_hwtstamp(struct net_device *netdev, struct sk_buff *skb)
797 {
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 12151 bytes --]
^ permalink raw reply
* Re: [PATCH net-next] bpf, doc: update list of architectures that do eBPF JIT
From: David Miller @ 2017-04-23 19:57 UTC (permalink / raw)
To: ast; +Cc: daniel, netdev
In-Reply-To: <20170423160100.1155435-1-ast@fb.com>
From: Alexei Starovoitov <ast@fb.com>
Date: Sun, 23 Apr 2017 09:01:00 -0700
> update the list and remove 'in the future' statement,
> since all still alive 64-bit architectures now do eBPF JIT.
>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Applied, thanks :)
^ permalink raw reply
* Re: [PATCH] brcm80211: brcmfmac: Ensure that incoming skb's are writable
From: Arend Van Spriel @ 2017-04-23 19:34 UTC (permalink / raw)
To: James Hughes
Cc: netdev, Franky Lin, Hante Meuleman, Kalle Valo, linux-wireless
In-Reply-To: <CAE_XsMKSO=P-h5AyPJy=59zDq=DmLFqBAX8Bqsofey=aAFD5dQ@mail.gmail.com>
On 21-4-2017 11:22, James Hughes wrote:
> On 20 April 2017 at 20:48, Arend van Spriel
> <arend.vanspriel@broadcom.com> wrote:
>> + linux-wireless
>>
>> On 4/20/2017 1:16 PM, James Hughes wrote:
>>>
>>> The driver was adding header information to incoming skb
>>> without ensuring the head was uncloned and hence writable.
>>>
>>> skb_cow_head has been used to ensure they are writable, however,
>>> this required some changes to error handling to ensure that
>>> if skb_cow_head failed it was not ignored.
>>>
>>> This really needs to be reviewed by someone who is more familiar
>>> with this code base to ensure any deallocation of skb's is
>>> still correct.
>>>
>>> Signed-off-by: James Hughes <james.hughes@raspberrypi.org>
>>> ---
>>> .../wireless/broadcom/brcm80211/brcmfmac/bcdc.c | 15 ++++++++--
>>> .../wireless/broadcom/brcm80211/brcmfmac/core.c | 23 +++++-----------
>>> .../broadcom/brcm80211/brcmfmac/fwsignal.c | 32
>>> +++++++++++++++++-----
>>> .../wireless/broadcom/brcm80211/brcmfmac/sdio.c | 7 ++++-
>>> 4 files changed, 51 insertions(+), 26 deletions(-)
>>>
>>
>> [...]
>>
>>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
>>> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
>>> index 5eaac13..08272e8 100644
>>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
>>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
>>> @@ -198,7 +198,7 @@ static netdev_tx_t brcmf_netdev_start_xmit(struct
>>> sk_buff *skb,
>>> int ret;
>>> struct brcmf_if *ifp = netdev_priv(ndev);
>>> struct brcmf_pub *drvr = ifp->drvr;
>>> - struct ethhdr *eh = (struct ethhdr *)(skb->data);
>>> + struct ethhdr *eh;
>>> brcmf_dbg(DATA, "Enter, bsscfgidx=%d\n", ifp->bsscfgidx);
>>> @@ -212,23 +212,14 @@ static netdev_tx_t brcmf_netdev_start_xmit(struct
>>> sk_buff *skb,
>>> }
>>> /* Make sure there's enough room for any header */
>>> - if (skb_headroom(skb) < drvr->hdrlen) {
>>> - struct sk_buff *skb2;
>>> -
>>> - brcmf_dbg(INFO, "%s: insufficient headroom\n",
>>> - brcmf_ifname(ifp));
>>> - drvr->bus_if->tx_realloc++;
>>> - skb2 = skb_realloc_headroom(skb, drvr->hdrlen);
>>> - dev_kfree_skb(skb);
>>> - skb = skb2;
>>> - if (skb == NULL) {
>>> - brcmf_err("%s: skb_realloc_headroom failed\n",
>>> - brcmf_ifname(ifp));
>>> - ret = -ENOMEM;
>>> - goto done;
>>> - }
>>
>>
>> What you are throwing away here is code that assures there is sufficient
>> headroom for protocol and bus layer in the tx path, because that is
>> determined by drvr->hdrlen. This is where the skb is handed to the driver so
>> if you could leave the functionality above *and* assure it is writeable that
>> would be the best solution as there is no need for all the other changes
>> down the tx path.
>
> The skb_cow_head function takes the required headroom as a parameter
> and will ensure that there is enough space, so I don't think this code
> segment is
> required or have I misunderstood what you mean here?
I looked more into what skb_cow_head() and skb_realloc_headroom()
actually do and it seems you are right.
> Is it safe to rely on the _cow_ being done here and not further down
> in the stack?
> Or at least checked further down in the stack. Previous comments from
> another patch
> requested that the _cow_ be done close to the actual addition of the
> header. I presume
> it is unlikely/impossible that the functions that add header
> information down the stack
> will be called without the above being done first?
It is safe. During probe sequence each layer in the driver stack
increments drvr->hdrlen with headroom it needs. So drvr->hdrlen will
indicate the total headroom needed when .start_xmit() is called. And
yes, the .start_xmit() is the single point of entry in the transmit
path. So the other locations where skb_push() is done are safe.
>>
>>> + ret = skb_cow_head(skb, drvr->hdrlen);
>>> + if (ret) {
>>
>>
>> So move the realloc code above here instead of simply freeing the skb.
>>
>>> + dev_kfree_skb_any(skb);
>>> + goto done;
>>> }
>>> + eh = (struct ethhdr *)(skb->data);
>>
>>
>> Now this is actually a separate fix so I would like a separate patch for it.
>>
>
> No problem, but see final paragraph below.
Let me see...
>> I have a RPi3 sitting on my desk so how can I replicate the issue. It was
>> something about broadcast/multicast traffic when using AP mode and a bridge,
>> right?
>>
>> Regards,
>> Arend
>
> See this issue for details on replication.
> https://github.com/raspberrypi/firmware/issues/673
>
> The bridge I use is setup using a similar procedure to that described
> here. https://www.raspberrypi.org/documentation/configuration/wireless/access-point.md
>
> I'm happy to pass over this work to you guys if you think it is
> appropriate, you are the
> experts on the codebase.
Sure. However, I must say you did an excellent job digging into the
issue and root causing it. We always were under the impression that we
could treat the skb as our own when handed to us in .start_xmit() callback.
Regards,
Arend
^ permalink raw reply
* Re: [PATCH 1/1] cfg80211: add return value validation
From: Johannes Berg @ 2017-04-23 18:41 UTC (permalink / raw)
To: Pan Bian, Jussi Kivilinna, Kalle Valo,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Pan Bian
In-Reply-To: <1492954023-8596-1-git-send-email-bianpan201603-9Onoh4P/yGk@public.gmane.org>
This is not a cfg80211 patch, please resend with the correct subject.
Thanks,
johannes
^ permalink raw reply
* Re: [PATCH net] ravb: Double free on error in ravb_start_xmit()
From: Sergei Shtylyov @ 2017-04-23 18:16 UTC (permalink / raw)
To: Dan Carpenter
Cc: David S. Miller, Kazuya Mizuguchi, Simon Horman, Yoshihiro Kaneko,
Masaru Nagai, Geert Uytterhoeven, Niklas Söderlund,
Philippe Reynes, netdev, linux-renesas-soc, kernel-janitors
In-Reply-To: <20170422104656.jjnooh3or4x3lbw3@mwanda>
Hello!
On 04/22/2017 01:46 PM, Dan Carpenter wrote:
> If skb_put_padto() fails then it frees the skb. I shifted that code
> up a bit to make my error handling a little simpler.
>
> Fixes: a0d2f20650e8 ("Renesas Ethernet AVB PTP clock driver")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Acked-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
MBR, Sergei
^ permalink raw reply
* [PATCH 1/1] drivers:net:ethernet:adi:bfin_mac: Use FIELD_SIZEOF defined kernel macro
From: Karim Eshapa @ 2017-04-23 18:02 UTC (permalink / raw)
To: davem
Cc: tremyfr, a, fw, clabbe.montjoie, edumazet, jarod,
adi-buildroot-devel, netdev, linux-kernel, Karim Eshapa
Use FIELD_SIZEOF defined kernel macro kernel.h
Signed-off-by: Karim Eshapa <karim.eshapa@gmail.com>
---
drivers/net/ethernet/adi/bfin_mac.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/adi/bfin_mac.c b/drivers/net/ethernet/adi/bfin_mac.c
index a9ac58c..60346e0 100644
--- a/drivers/net/ethernet/adi/bfin_mac.c
+++ b/drivers/net/ethernet/adi/bfin_mac.c
@@ -452,10 +452,14 @@ static irqreturn_t bfin_mac_wake_interrupt(int irq, void *dev_id)
static void bfin_mac_ethtool_getdrvinfo(struct net_device *dev,
struct ethtool_drvinfo *info)
{
- strlcpy(info->driver, KBUILD_MODNAME, sizeof(info->driver));
- strlcpy(info->version, DRV_VERSION, sizeof(info->version));
- strlcpy(info->fw_version, "N/A", sizeof(info->fw_version));
- strlcpy(info->bus_info, dev_name(&dev->dev), sizeof(info->bus_info));
+ strlcpy(info->driver, KBUILD_MODNAME, FIELD_SIZEOF(
+ struct ethtool_drvinfo, driver));
+ strlcpy(info->version, DRV_VERSION, FIELD_SIZEOF(
+ struct ethtool_drvinfo, version));
+ strlcpy(info->fw_version, "N/A", FIELD_SIZEOF(
+ struct ethtool_drvinfo, fw_version));
+ strlcpy(info->bus_info, dev_name(&dev->dev), FIELD_SIZEOF(
+ struct ethtool_drvinfo, bus_info));
}
static void bfin_mac_ethtool_getwol(struct net_device *dev,
@@ -785,7 +789,7 @@ static int bfin_mac_hwtstamp_get(struct net_device *netdev,
struct bfin_mac_local *lp = netdev_priv(netdev);
return copy_to_user(ifr->ifr_data, &lp->stamp_cfg,
- sizeof(lp->stamp_cfg)) ?
+ FILD_SIZEOF(struct bfin_mac_local, stamp_cfg)) ?
-EFAULT : 0;
}
--
2.7.4
^ permalink raw reply related
* Re: [PATCH iproute2 net 3/8] tc/pedit: Introduce 'add' operation
From: Jamal Hadi Salim @ 2017-04-23 17:44 UTC (permalink / raw)
To: Amir Vadai, Stephen Hemminger; +Cc: netdev, Or Gerlitz
In-Reply-To: <20170423125356.1298-4-amir@vadai.me>
Thanks for the excellent work.
On 17-04-23 08:53 AM, Amir Vadai wrote:
> This command could be useful to increase/decrease fields value.
>
Does this contradict the "retain" feature? Example rule to
retain the second nibble but set the first to 0xA
(essentially it X-ORs):
tc filter add dev lo parent ffff: protocol ip prio 10 \
u32 match ip dst 127.0.0.1/32 flowid 1:1 \
action pedit munge offset 0 u8 set 0x0A retain 0xF0
Mostly curious about hardware handling.
cheers,
jamal
^ permalink raw reply
* Re: [PATCH net-next] bpf, doc: update list of architectures that do eBPF JIT
From: Daniel Borkmann @ 2017-04-23 17:39 UTC (permalink / raw)
To: Alexei Starovoitov, David S . Miller; +Cc: netdev
In-Reply-To: <20170423160100.1155435-1-ast@fb.com>
On 04/23/2017 06:01 PM, Alexei Starovoitov wrote:
> update the list and remove 'in the future' statement,
> since all still alive 64-bit architectures now do eBPF JIT.
>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
> mips64 is the only 'still alive' 64-bit arch without eBPF JIT :)
(... and work in progress by cavium folks. ;))
^ permalink raw reply
* [PATCH v2] net: can: usb: gs_usb: Fix buffer on stack
From: Maksim Salau @ 2017-04-23 17:31 UTC (permalink / raw)
To: Wolfgang Grandegger, Marc Kleine-Budde, Maximilian Schneider,
Hubert Denkmair, Wolfram Sang, Ethan Zonca, linux-can, netdev,
Fabio Estevam
Cc: Maksim Salau
In-Reply-To: <CAOMZO5DLdR00FY01r2z=RvSrn4vNEZ+Z3OS0KWB95DGt=3fh2w@mail.gmail.com>
Allocate buffers on HEAP instead of STACK for local structures
that are to be sent using usb_control_msg().
Signed-off-by: Maksim Salau <maksim.salau@gmail.com>
---
Changes in v2:
dropped redundant assignment.
drivers/net/can/usb/gs_usb.c | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
index 300349f..eecee7f 100644
--- a/drivers/net/can/usb/gs_usb.c
+++ b/drivers/net/can/usb/gs_usb.c
@@ -739,13 +739,18 @@ static const struct net_device_ops gs_usb_netdev_ops = {
static int gs_usb_set_identify(struct net_device *netdev, bool do_identify)
{
struct gs_can *dev = netdev_priv(netdev);
- struct gs_identify_mode imode;
+ struct gs_identify_mode *imode;
int rc;
+ imode = kmalloc(sizeof(*imode), GFP_KERNEL);
+
+ if (!imode)
+ return -ENOMEM;
+
if (do_identify)
- imode.mode = GS_CAN_IDENTIFY_ON;
+ imode->mode = GS_CAN_IDENTIFY_ON;
else
- imode.mode = GS_CAN_IDENTIFY_OFF;
+ imode->mode = GS_CAN_IDENTIFY_OFF;
rc = usb_control_msg(interface_to_usbdev(dev->iface),
usb_sndctrlpipe(interface_to_usbdev(dev->iface),
@@ -755,10 +760,12 @@ static int gs_usb_set_identify(struct net_device *netdev, bool do_identify)
USB_RECIP_INTERFACE,
dev->channel,
0,
- &imode,
- sizeof(imode),
+ imode,
+ sizeof(*imode),
100);
+ kfree(imode);
+
return (rc > 0) ? 0 : rc;
}
--
2.9.3
^ permalink raw reply related
* Re: [Intel-wired-lan] NFS over NAT causes e1000e transmit hangs
From: Eric Dumazet @ 2017-04-23 17:24 UTC (permalink / raw)
To: Florian Fainelli; +Cc: Neftin, Sasha, netdev, intel-wired-lan
In-Reply-To: <09a5327d-0cf9-c4ed-5383-2f02c658437e@gmail.com>
On Sun, 2017-04-23 at 10:08 -0700, Florian Fainelli wrote:
>
> On 04/22/2017 11:46 PM, Neftin, Sasha wrote:
> > On 4/20/2017 00:15, Florian Fainelli wrote:
> >> On 04/19/2017 01:52 AM, Neftin, Sasha wrote:
> >>> On 4/18/2017 22:05, Florian Fainelli wrote:
> >>>> On 04/18/2017 12:03 PM, Eric Dumazet wrote:
> >>>>> On Tue, 2017-04-18 at 11:18 -0700, Florian Fainelli wrote:
> >>>>>> Hi,
> >>>>>>
> >>>>>> I am using NFS over a NAT with two e1000e adapters and with eth1
> >>>>>> being
> >>>>>> the LAN interface and eth0 the WAN interface. The kernel is Ubuntu's
> >>>>>> 16.10 kernel: 4.8.0-46-generic. The device doing NAT over NFS is just
> >>>>>> mounting a remote folder and doing normal execution/file accesses.
> >>>>>> It's
> >>>>>> enough to untar a file from this device onto a NFS share to expose
> >>>>>> the
> >>>>>> problem.
> >>>>>>
> >>>>>> The transmit hangs look like the ones below, doing a rmmod/insmod
> >>>>>> does
> >>>>>> not help eliminated the problem, nor does a power cycle. Stopping the
> >>>>>> NFS over NAT definitively does let the adapter recover.
> >>>>> Is this NFS over TCP or UDP ?
> >>>> This is NFS over TCP mounted with the following:
> >>>>
> >>>> type nfs
> >>>> (rw,relatime,vers=3,rsize=1048576,wsize=1048576,namlen=255,hard,proto=tcp,port=2049,timeo=70,retrans=3,sec=sys,local_lock=none,addr=X.X.X.X)
> >>>>
> >>>>
> >>>>
> >>>> Thanks Eric!
> >>> Please, try disable TCP segmentation offload: ethtool -K <adapter>
> >>> tso off.
> >> I am not able to reproduce the hangs with TSO turned off. Is there a
> >> specific patch you would want me to try?
> >
> > Please, work with TSO turned off so. There is no patch for this specific
> > problem.
>
> OK, are not we interested in somehow being able to identify such
> problematic packets coming from the networking stack and force not using
> TSO for those? Would an acceptable solution be to force the disabling of
> TSO for this specific NIC model (provided it is some kind of HW bug)?
>
> NB: I understand this is very old hardware for you at Intel, but
> conversely, it is very widespread, and chances of people running into
> similar issues are pretty high, so fixing it once would de-facto lower
> the amount of support you'd have to provide in the future.
Indeed it is very odd to disable TSO, especially if the problem only
shows up with NAT.
We probably have a nasty bug somewhere, or we might be able to have a
work around some hardware 'feature'.
^ permalink raw reply
* Re: [PATCH iproute2 1/1] actions: Add support for user cookies
From: Jamal Hadi Salim @ 2017-04-23 17:18 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, Lucas Bates
In-Reply-To: <20170423091109.79d47f6b@xeon-e3>
On 17-04-23 12:11 PM, Stephen Hemminger wrote:
> On Sat, 22 Apr 2017 08:36:23 -0400
>
> Applied. Please update man page as well.
>
>
Will do.
cheers,
jamal
^ permalink raw reply
* Re: [PATCH] net: can: usb: gs_usb: Fix buffer on stack
From: Maksim Salau @ 2017-04-23 17:18 UTC (permalink / raw)
To: Fabio Estevam
Cc: Wolfgang Grandegger, Marc Kleine-Budde, Maximilian Schneider,
Hubert Denkmair, Wolfram Sang, Ethan Zonca, linux-can,
netdev@vger.kernel.org
In-Reply-To: <CAOMZO5DLdR00FY01r2z=RvSrn4vNEZ+Z3OS0KWB95DGt=3fh2w@mail.gmail.com>
> > + struct gs_identify_mode *imode = NULL;
>
> No need to assign imode to NULL here.
Thanks, Fabio.
I'll fix this in v2.
Maksim
^ permalink raw reply
* [PATCH net-next 1/1] net sched actions: Complete the JUMPX opcode
From: Jamal Hadi Salim @ 2017-04-23 17:17 UTC (permalink / raw)
To: davem
Cc: netdev, jiri, xiyou.wangcong, daniel, alexei.starovoitov,
Jamal Hadi Salim
From: Jamal Hadi Salim <jhs@mojatatu.com>
per discussion at netconf/netdev:
When we have an action that is capable of branching (example a policer),
we can achieve a continuation of the action graph by programming a
"continue" where we find an exact replica of the same filter rule with a lower
priority and the remainder of the action graph. When you have 100s of thousands
of filters which require such a feature it gets very inefficient to do two
lookups.
This patch completes a leftover feature of action codes. Its time has come.
Example below where a user labels packets with a different skbmark on ingress
of a port depending on whether they have/not exceeded the configured rate.
This mark is then used to make further decisions on some egress port.
#rate control, very low so we can easily see the effect
sudo $TC actions add action police rate 1kbit burst 90k \
conform-exceed pipe/jump 2 index 10
# skbedit index 11 will be used if the user conforms
sudo $TC actions add action skbedit mark 11 ok index 11
# skbedit index 12 will be used if the user does not conform
sudo $TC actions add action skbedit mark 12 ok index 12
#lets bind the user ..
sudo $TC filter add dev $ETH parent ffff: protocol ip prio 8 u32 \
match ip dst 127.0.0.8/32 flowid 1:10 \
action police index 10 \
action skbedit index 11 \
action skbedit index 12
#run a ping -f and see what happens..
#
jhs@foobar:~$ sudo $TC -s filter ls dev $ETH parent ffff: protocol ip
filter pref 8 u32
filter pref 8 u32 fh 800: ht divisor 1
filter pref 8 u32 fh 800::800 order 2048 key ht 800 bkt 0 flowid 1:10 (rule hit 2800 success 1005)
match 7f000008/ffffffff at 16 (success 1005 )
action order 1: police 0xa rate 1Kbit burst 23440b mtu 2Kb action pipe/jump 2 overhead 0b
ref 2 bind 1 installed 207 sec used 122 sec
Action statistics:
Sent 84420 bytes 1005 pkt (dropped 0, overlimits 721 requeues 0)
backlog 0b 0p requeues 0
action order 2: skbedit mark 11 pass
index 11 ref 2 bind 1 installed 204 sec used 122 sec
Action statistics:
Sent 60564 bytes 721 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
action order 3: skbedit mark 12 pass
index 12 ref 2 bind 1 installed 201 sec used 122 sec
Action statistics:
Sent 23856 bytes 284 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
Not bad, about 28% non-conforming packets..
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
net/sched/act_api.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 257360f..7f2cd70 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -428,24 +428,49 @@ static struct tc_action_ops *tc_lookup_action(struct nlattr *kind)
return res;
}
+/*TCA_ACT_MAX_PRIO is 32, there count upto 32 */
+#define TCA_ACT_MAX_PRIO_MASK 0x1FF
int tcf_action_exec(struct sk_buff *skb, struct tc_action **actions,
int nr_actions, struct tcf_result *res)
{
int ret = -1, i;
+ u32 jmp_prgcnt = 0;
+ u32 jmp_ttl = TCA_ACT_MAX_PRIO; /*matches actions per filter */
if (skb_skip_tc_classify(skb))
return TC_ACT_OK;
+restart_act_graph:
for (i = 0; i < nr_actions; i++) {
const struct tc_action *a = actions[i];
+ if (jmp_prgcnt > 0) {
+ jmp_prgcnt -= 1;
+ continue;
+ }
repeat:
ret = a->ops->act(skb, a, res);
if (ret == TC_ACT_REPEAT)
goto repeat; /* we need a ttl - JHS */
+
+ if (ret & TC_ACT_JUMP) {
+ jmp_prgcnt = ret & TCA_ACT_MAX_PRIO_MASK;
+ if (!jmp_prgcnt || (jmp_prgcnt > nr_actions)) {
+ /* faulty opcode, stop pipeline */
+ return TC_ACT_OK;
+ } else {
+ jmp_ttl -= 1;
+ if (jmp_ttl > 0)
+ goto restart_act_graph;
+ else /* faulty graph, stop pipeline */
+ return TC_ACT_OK;
+ }
+ }
+
if (ret != TC_ACT_PIPE)
break;
}
+
return ret;
}
EXPORT_SYMBOL(tcf_action_exec);
--
1.9.1
^ permalink raw reply related
* Re: [Intel-wired-lan] NFS over NAT causes e1000e transmit hangs
From: Florian Fainelli @ 2017-04-23 17:08 UTC (permalink / raw)
To: Neftin, Sasha, Eric Dumazet; +Cc: netdev, intel-wired-lan
In-Reply-To: <81dbcf7d-52f4-4dc0-2355-27198685da7b@intel.com>
On 04/22/2017 11:46 PM, Neftin, Sasha wrote:
> On 4/20/2017 00:15, Florian Fainelli wrote:
>> On 04/19/2017 01:52 AM, Neftin, Sasha wrote:
>>> On 4/18/2017 22:05, Florian Fainelli wrote:
>>>> On 04/18/2017 12:03 PM, Eric Dumazet wrote:
>>>>> On Tue, 2017-04-18 at 11:18 -0700, Florian Fainelli wrote:
>>>>>> Hi,
>>>>>>
>>>>>> I am using NFS over a NAT with two e1000e adapters and with eth1
>>>>>> being
>>>>>> the LAN interface and eth0 the WAN interface. The kernel is Ubuntu's
>>>>>> 16.10 kernel: 4.8.0-46-generic. The device doing NAT over NFS is just
>>>>>> mounting a remote folder and doing normal execution/file accesses.
>>>>>> It's
>>>>>> enough to untar a file from this device onto a NFS share to expose
>>>>>> the
>>>>>> problem.
>>>>>>
>>>>>> The transmit hangs look like the ones below, doing a rmmod/insmod
>>>>>> does
>>>>>> not help eliminated the problem, nor does a power cycle. Stopping the
>>>>>> NFS over NAT definitively does let the adapter recover.
>>>>> Is this NFS over TCP or UDP ?
>>>> This is NFS over TCP mounted with the following:
>>>>
>>>> type nfs
>>>> (rw,relatime,vers=3,rsize=1048576,wsize=1048576,namlen=255,hard,proto=tcp,port=2049,timeo=70,retrans=3,sec=sys,local_lock=none,addr=X.X.X.X)
>>>>
>>>>
>>>>
>>>> Thanks Eric!
>>> Please, try disable TCP segmentation offload: ethtool -K <adapter>
>>> tso off.
>> I am not able to reproduce the hangs with TSO turned off. Is there a
>> specific patch you would want me to try?
>
> Please, work with TSO turned off so. There is no patch for this specific
> problem.
OK, are not we interested in somehow being able to identify such
problematic packets coming from the networking stack and force not using
TSO for those? Would an acceptable solution be to force the disabling of
TSO for this specific NIC model (provided it is some kind of HW bug)?
NB: I understand this is very old hardware for you at Intel, but
conversely, it is very widespread, and chances of people running into
similar issues are pretty high, so fixing it once would de-facto lower
the amount of support you'd have to provide in the future.
Thanks
--
Florian
^ permalink raw reply
* Re: [PATCH iproute2] iplink: Expose IFLA_*_FWMARK attributes for supported link types
From: Stephen Hemminger @ 2017-04-23 16:16 UTC (permalink / raw)
To: Craig Gallek; +Cc: netdev
In-Reply-To: <20170421181453.166316-1-kraigatgoog@gmail.com>
On Fri, 21 Apr 2017 14:14:53 -0400
Craig Gallek <kraigatgoog@gmail.com> wrote:
> From: Craig Gallek <kraig@google.com>
>
> This attribute allows the administrator to adjust the packet marking
> attribute of tunnels that support policy based routing.
>
> Signed-off-by: Craig Gallek <kraig@google.com>
Applied to net-next. Since the link attributes are not in 4.11
^ 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