* Re: [PATCH net-next 6/9] net: hns3: Add MDIO support to HNS3 Ethernet driver for hip08 SoC
From: Andrew Lunn @ 2017-06-11 2:43 UTC (permalink / raw)
To: Florian Fainelli
Cc: Salil Mehta, davem, yisen.zhuang, huangdaode, lipeng321,
mehta.salil.lnk, netdev, linux-kernel, linuxarm
In-Reply-To: <ba416774-9ccc-e6b6-3518-fb492b8b2858@gmail.com>
> > +int hclge_mac_mdio_config(struct hclge_dev *hdev)
> > +{
> > + struct hclge_mac *mac = &hdev->hw.mac;
> > + struct mii_bus *mdio_bus;
> > + struct net_device *ndev = &mac->ndev;
> > + struct phy_device *phy;
> > + bool is_c45;
> > + int ret;
> > +
> > + if (hdev->hw.mac.phy_addr >= PHY_MAX_ADDR)
> > + return 0;
> > +
> > + if (hdev->hw.mac.phy_if == PHY_INTERFACE_MODE_NA)
> > + return 0;
> > + else if (mac->phy_if == PHY_INTERFACE_MODE_SGMII)
> > + is_c45 = 0;
> > + else if (mac->phy_if == PHY_INTERFACE_MODE_XGMII)
> > + is_c45 = 1;
> > + else
> > + return -ENODATA;
>
> Can you consider using a switch () case statement here?
Does this concept even make sense? The Marvell 10G phy will use SGMII
for 10/100/1000Mbs and swap to XGMII for 10Gbps. It however stays a
c45 device all the time.
In general, i don't think PHY mode is related to C22/C45.
Andrew
^ permalink raw reply
* RE: [for-next 4/6] net/mlx5: FPGA, Add basic support for Innova
From: Ilan Tayari @ 2017-06-11 5:59 UTC (permalink / raw)
To: Jason Gunthorpe, Saeed Mahameed
Cc: Alexei Starovoitov, David S. Miller, Doug Ledford,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
jsorensen-b10kYP2dOMg@public.gmane.org, Andy Shevchenko,
linux-fpga-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Alan Tull,
yi1.li-VuQAYsv1563Yd54FQh9/CA@public.gmane.org, Boris Pismenny
In-Reply-To: <20170607192132.GA10929-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> -----Original Message-----
> From: Jason Gunthorpe [mailto:jgunthorpe@obsidianresearch.com]
> Subject: Re: [for-next 4/6] net/mlx5: FPGA, Add basic support for Innova
>
> > > This keep getting more ugly :(
> > >
> > > What about security? What if user space sends some raw packets to the
> > > FPGA - can it reprogram the ISPEC settings or worse?
> > >
> >
> > No such thing. This QP is only for internal driver/HW communications,
> > as it is faster from the existing command interface.
> > it is not meant to be exposed for any raw user space usages at all,
> > without proper standard API adapter of course.
>
> I'm not asking about the QP, I'm asking what happens after the NIC
> part. You use ROCE packets to control the FPGA. What prevents
> userspace from forcibly constructing roce packets and sending them to
> the FPGA. How does the FPGA know for certain the packet came from the
> kernel QP and not someplace else.
>
> This is especially true for mlx nics as there are many raw packet
> bypass mechanisms available to userspace.
Hi Jason,
The device uses internal signaling that ensures that no entity other than the mlx5 driver can talk over the FPGA channel.
This is also the reason why this is not a "ULP in a driver", but rather an internal bus that happens to use some of our existing HW features.
As explained earlier, this "bus" is an internal device implementation issue, and has nothing to do with the network or RDMA stack.
Ilan.
^ permalink raw reply
* Re: [PATCH net v3] net: ipmr: Fix some mroute forwarding issues in vrf's
From: Yotam Gigi @ 2017-06-11 6:12 UTC (permalink / raw)
To: Donald Sharp, netdev, dsahern, Thomas.Winter, nikolay, idosch,
roopa
In-Reply-To: <20170610203017.4085-1-sharpd@cumulusnetworks.com>
On 06/10/2017 11:30 PM, Donald Sharp wrote:
> This patch fixes two issues:
>
> 1) When forwarding on *,G mroutes that are in a vrf, the
> kernel was dropping information about the actual incoming
> interface when calling ip_mr_forward from ip_mr_input.
> This caused ip_mr_forward to send the multicast packet
> back out the incoming interface. Fix this by
> modifying ip_mr_forward to be handed the correctly
> resolved dev.
>
> 2) When a unresolved cache entry is created we store
> the incoming skb on the unresolved cache entry and
> upon mroute resolution from the user space daemon,
> we attempt to forward the packet. Again we were
> not resolving to the correct incoming device for
> a vrf scenario, before calling ip_mr_forward.
> Fix this by resolving to the correct interface
> and calling ip_mr_forward with the result.
>
> Fixes: e58e41596811 ("net: Enable support for VRF with ipv4 multicast")
> Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
> ---
> v2: Fixed title
> v3: Addressed Review comments by Andrew Lunn and David Ahern
>
> net/ipv4/ipmr.c | 32 +++++++++++++++-----------------
> 1 file changed, 15 insertions(+), 17 deletions(-)
>
> diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
> index 551de4d..09368a1 100644
> --- a/net/ipv4/ipmr.c
> +++ b/net/ipv4/ipmr.c
> @@ -101,8 +101,8 @@ static struct mr_table *ipmr_new_table(struct net *net, u32 id);
> static void ipmr_free_table(struct mr_table *mrt);
>
> static void ip_mr_forward(struct net *net, struct mr_table *mrt,
> - struct sk_buff *skb, struct mfc_cache *cache,
> - int local);
> + struct net_device *dev, struct sk_buff *skb,
> + struct mfc_cache *cache, int local);
> static int ipmr_cache_report(struct mr_table *mrt,
> struct sk_buff *pkt, vifi_t vifi, int assert);
> static int __ipmr_fill_mroute(struct mr_table *mrt, struct sk_buff *skb,
> @@ -988,7 +988,7 @@ static void ipmr_cache_resolve(struct net *net, struct mr_table *mrt,
>
> rtnl_unicast(skb, net, NETLINK_CB(skb).portid);
> } else {
> - ip_mr_forward(net, mrt, skb, c, 0);
> + ip_mr_forward(net, mrt, skb->dev, skb, c, 0);
> }
> }
> }
> @@ -1073,7 +1073,7 @@ static int ipmr_cache_report(struct mr_table *mrt,
>
> /* Queue a packet for resolution. It gets locked cache entry! */
> static int ipmr_cache_unresolved(struct mr_table *mrt, vifi_t vifi,
> - struct sk_buff *skb)
> + struct sk_buff *skb, struct net_device *dev)
> {
> const struct iphdr *iph = ip_hdr(skb);
> struct mfc_cache *c;
> @@ -1130,6 +1130,10 @@ static int ipmr_cache_unresolved(struct mr_table *mrt, vifi_t vifi,
> kfree_skb(skb);
> err = -ENOBUFS;
> } else {
> + if (dev) {
> + skb->dev = dev;
> + skb->skb_iif = dev->ifindex;
> + }
> skb_queue_tail(&c->mfc_un.unres.unresolved, skb);
> err = 0;
> }
> @@ -1828,10 +1832,10 @@ static int ipmr_find_vif(struct mr_table *mrt, struct net_device *dev)
>
> /* "local" means that we should preserve one skb (for local delivery) */
> static void ip_mr_forward(struct net *net, struct mr_table *mrt,
> - struct sk_buff *skb, struct mfc_cache *cache,
> - int local)
> + struct net_device *dev, struct sk_buff *skb,
> + struct mfc_cache *cache, int local)
> {
> - int true_vifi = ipmr_find_vif(mrt, skb->dev);
> + int true_vifi = ipmr_find_vif(mrt, dev);
> int psend = -1;
> int vif, ct;
>
> @@ -1853,13 +1857,7 @@ static void ip_mr_forward(struct net *net, struct mr_table *mrt,
> }
>
> /* Wrong interface: drop packet and (maybe) send PIM assert. */
> - if (mrt->vif_table[vif].dev != skb->dev) {
> - struct net_device *mdev;
> -
> - mdev = l3mdev_master_dev_rcu(mrt->vif_table[vif].dev);
> - if (mdev == skb->dev)
> - goto forward;
> -
> + if (mrt->vif_table[vif].dev != dev) {
> if (rt_is_output_route(skb_rtable(skb))) {
> /* It is our own packet, looped back.
> * Very complicated situation...
> @@ -2053,7 +2051,7 @@ int ip_mr_input(struct sk_buff *skb)
> read_lock(&mrt_lock);
> vif = ipmr_find_vif(mrt, dev);
> if (vif >= 0) {
> - int err2 = ipmr_cache_unresolved(mrt, vif, skb);
> + int err2 = ipmr_cache_unresolved(mrt, vif, skb, dev);
> read_unlock(&mrt_lock);
>
> return err2;
> @@ -2064,7 +2062,7 @@ int ip_mr_input(struct sk_buff *skb)
> }
>
> read_lock(&mrt_lock);
> - ip_mr_forward(net, mrt, skb, cache, local);
> + ip_mr_forward(net, mrt, dev, skb, cache, local);
> read_unlock(&mrt_lock);
>
> if (local)
> @@ -2238,7 +2236,7 @@ int ipmr_get_route(struct net *net, struct sk_buff *skb,
> iph->saddr = saddr;
> iph->daddr = daddr;
> iph->version = 0;
> - err = ipmr_cache_unresolved(mrt, vif, skb2);
> + err = ipmr_cache_unresolved(mrt, vif, skb2, dev);
> read_unlock(&mrt_lock);
> rcu_read_unlock();
> return err;
Thanks Donald!
Reviewed-by: Yotam Gigi <yotamg@mellanox.com>
^ permalink raw reply
* Re: ARM GLX Khadas VIM Pro - Ethernet detected as only 10Mbps and stalled after some traffic
From: crow @ 2017-06-11 6:31 UTC (permalink / raw)
To: Andrew Lunn; +Cc: netdev, linux-amlogic
In-Reply-To: <20170610152741.GB26740@lunn.ch>
Hi Andrew,
On Sat, Jun 10, 2017 at 5:27 PM, Andrew Lunn <andrew@lunn.ch> wrote:
>> Also what Martin Blumenstingl wrote is following which is also crucial
>> for fixing the issue:
>> Amlogic has given their ethernet PHY driver some updates [2], it now
>> includes wake-on-lan, and they now have an internal_phy_read_status
>> which uses reset_internal_phy if there's a link and some error counter
>> exceeds some magic value.
>
> Hi Crow
>
> You could probably just drop the Amlogic driver into mainline and see
> if it works better. If that solves your problem, we can look at
> merging the changes.
>
> Andrew
Thank your for the suggestion, and thanks Martin to explaining me over
IRC what actually I should do.
I recompiled mainline kernel 4.12-rc4 with the Amlogic driver:
replaced drivers/net/phy/meson-gxl.c with
https://github.com/khadas/linux/blob/ubuntu-4.9/drivers/amlogic/ethernet/phy/amlogic.c
But this did not solve the issue. As soon as i start git clone i lose
network connection to device (no session timeout/disconnect this time,
but I am unable to reconnect over SSH or to get OK ping replay back).
Here are the tests:
Linux khadasvimpro 4.12.0-rc4-4-ARCH #1 SMP Sun Jun 11 03:39:21 CEST
2017 aarch64 GNU/Linux
# modinfo meson_gxl
filename:
/lib/modules/4.12.0-rc4-4-ARCH/kernel/drivers/net/phy/meson-gxl.ko.gz
license: GPL
author: Neil Armstrong <narmstrong@baylibre.com>
author: Baoqi wang
description: Amlogic Meson GXL Internal PHY driver
alias: mdio:0000000110000001010001000000????
depends:
intree: Y
vermagic: 4.12.0-rc4-4-ARCH SMP mod_unload aarch64
#
# mii-tool -vvv eth0
Using SIOCGMIIPHY=0x8947
eth0: negotiated 1000baseT-HD flow-control, link ok
registers for MII PHY 8:
1000 782d 0181 4400 01e1 c1e1 000f 2001
ffff ffff ffff ffff ffff ffff ffff ffff
0040 0002 40e8 5400 1c1c 0000 0000 aaaa
fff0 ffff 0000 040a 1407 004a 0000 105a
product info: vendor 00:60:51, model 0 rev 0
basic mode: autonegotiation enabled
basic status: autonegotiation complete, link ok
capabilities: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD
10baseT-FD 10baseT-HD
advertising: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD
10baseT-FD 10baseT-HD
link partner: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD
10baseT-FD 10baseT-HD
$
over SSH startet following but it stall already at 0%:
$ git clone git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git
Cloning into 'linux-stable'...
remote: Counting objects: 5948690, done.
remote: Compressing objects: 100% (124799/124799), done.
Receiving objects: 0% (11668/5948690), 2.27 MiB | 4.52 MiB/s
shows timeout while trying to sync with NTP server:
# journalctl -f
systemd-timesyncd[299]: Timed out waiting for reply from
83.68.137.76:123 (2.at.pool.ntp.org).
systemd-timesyncd[299]: Timed out waiting for reply from
86.59.113.114:123 (2.at.pool.ntp.org).
while still not working dump the register:
# mii-tool -vvv eth0
Using SIOCGMIIPHY=0x8947
eth0: negotiated 1000baseT-HD flow-control, link ok
registers for MII PHY 8:
1000 782d 0181 4400 01e1 c1e1 000d 2001
ffff ffff ffff ffff ffff ffff ffff ffff
0040 0002 40e8 5400 1c1c 0000 0000 aaaa
fff0 ffff 0000 040a 1407 0000 0000 105a
product info: vendor 00:60:51, model 0 rev 0
basic mode: autonegotiation enabled
basic status: autonegotiation complete, link ok
capabilities: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD
10baseT-FD 10baseT-HD
advertising: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD
10baseT-FD 10baseT-HD
link partner: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD
10baseT-FD 10baseT-HD
#
# ifconfig eth0 down && ifconfig eth0 up
# dmesg -T | tail -n 10
[Sun Jun 11 07:40:34 2017] meson8b-dwmac c9410000.ethernet eth0:
device MAC address 00:15:18:01:81:31
[Sun Jun 11 07:40:34 2017] random: crng init done
[Sun Jun 11 07:40:34 2017] Meson GXL Internal PHY 0.e40908ff:08:
attached PHY driver [Meson GXL Internal PHY]
(mii_bus:phy_addr=0.e40908ff:08, irq=-1)
[Sun Jun 11 07:40:34 2017] meson8b-dwmac c9410000.ethernet eth0: PTP
not supported by HW
[Sun Jun 11 07:40:36 2017] brcmfmac: brcmf_c_preinit_dcmds: Firmware
version = wl0: Mar 1 2015 07:29:38 version 7.45.18 (r538002) FWID
01-6a2c8ad4
[Sun Jun 11 07:40:36 2017] meson8b-dwmac c9410000.ethernet eth0: Link
is Up - 100Mbps/Full - flow control off
[Sun Jun 11 07:41:23 2017] EXT4-fs (mmcblk1p1): mounted filesystem
with ordered data mode. Opts: (null)
[Sun Jun 11 07:54:28 2017] Meson GXL Internal PHY 0.e40908ff:08:
attached PHY driver [Meson GXL Internal PHY]
(mii_bus:phy_addr=0.e40908ff:08, irq=-1)
[Sun Jun 11 07:54:28 2017] meson8b-dwmac c9410000.ethernet eth0: PTP
not supported by HW
[Sun Jun 11 07:54:30 2017] meson8b-dwmac c9410000.ethernet eth0: Link
is Up - 100Mbps/Full - flow control off
#
then I took eth0 and wlan0 up (same ArchLinuxArm with same mainline
kernel same place where the files are stored eMMC) and this was
working so it should not be the ArchLinuxArm which makes problem, and
did same git clone as with custom Amlogic kernel (3.1.4 and 4.9.26
under Khadas Ubuntu image) and test was successfully:
$ git clone git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git
Cloning into 'linux-stable'...
remote: Counting objects: 5948690, done.
remote: Compressing objects: 100% (124799/124799), done.
remote: Total 5948690 (delta 427756), reused 549521 (delta 425675)
Receiving objects: 100% (5948690/5948690), 1.21 GiB | 4.94 MiB/s, done.
Resolving deltas: 100% (4961965/4961965), done.
Checking out files: 100% (59844/59844), done.
$
Regards,
^ permalink raw reply
* Re: [PATCH v2] brcmfmac: Fix glom_skb leak in brcmf_sdiod_recv_chain
From: Arend van Spriel @ 2017-06-11 8:08 UTC (permalink / raw)
To: Peter Housel
Cc: Andy Shevchenko, Franky Lin, Hante Meuleman, Kalle Valo,
Pieter-Paul Giesberts, Christian Daudt, Florian Fainelli,
Florian Westphal, Martin Blumenstingl,
open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER,
open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER,
open list:NETWORKING DRIVERS, open list
In-Reply-To: <705684CB-D155-45DC-8146-157A536F9FBA-HInyCGIudOg@public.gmane.org>
On 11-06-17 02:18, Peter Housel wrote:
>
>> On Jun 10, 2017, at 12:27 PM, Arend van Spriel <arend.vanspriel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> wrote:
>>
>> On 03-06-17 17:36, Andy Shevchenko wrote:
>>> On Sat, Jun 3, 2017 at 1:29 AM, Peter S. Housel <housel-HInyCGIudOg@public.gmane.org> wrote:
>>>> An earlier change to this function (3bdae810721b) fixed a leak in the
>>>> case of an unsuccessful call to brcmf_sdiod_buffrw(). However, the
>>>> glom_skb buffer, used for emulating a scattering read, is never used
>>>> or referenced after its contents are copied into the destination
>>>> buffers, and therefore always needs to be freed by the end of the
>>>> function.
>>
>> [snip]
>>
>>>> + skb_queue_walk(pktq, skb) {
>>>> + memcpy(skb->data, glom_skb->data, skb->len);
>>>> + skb_pull(glom_skb, skb->len);
>>>> + }
>>>> }
>>>
>>>> + brcmu_pkt_buf_free_skb(glom_skb);
>>>
>>> Can we just add this one line instead or I'm missing something?
>>
>> I guess. We don't want to walk the packet queue if glom_skb is not
>> carrying data due to brcmf_sdiod_buffrw() failure.
>>
>> So I would go with the patch below as brcmu_pkt_buf_free_skb() simply
>> ignores null pointer.
>
> I’m fine with this, or indeed most of the other proposed solutions. The important thing is that the leak is fixed; in the driver's current state I was able to run our wearable device out of memory in just over 20 seconds running iperf.
Sure. The reason behind the suggestion from Franky was to get rid of the
label inside branch and I agree with that. To address Andy's comment I
think my proposal should tackle that.
Just out of curiosity, we added the broken-sg-support thing for OMAP
platform. So what platform/mmc-host are you using. I try to keep an
overview where this workaround is needed.
Regards,
Arend
^ permalink raw reply
* Re: [PATCH 0/6] Constant Time Memory Comparisons Are Important
From: Kalle Valo @ 2017-06-11 8:13 UTC (permalink / raw)
To: Jason A. Donenfeld
Cc: linux-kernel, kernel-hardening, Anna Schumaker, David Howells,
David Safford, David S. Miller, Gilad Ben-Yossef,
Greg Kroah-Hartman, Gustavo Padovan, J. Bruce Fields, Jeff Layton,
Johan Hedberg, Johannes Berg, Marcel Holtmann, Mimi Zohar,
Trond Myklebust, keyrings, linux-bluetooth, linux-nfs,
linux-wireless
In-Reply-To: <20170610025912.6499-1-Jason@zx2c4.com>
"Jason A. Donenfeld" <Jason@zx2c4.com> writes:
> Whenever you're comparing two MACs, it's important to do this using
> crypto_memneq instead of memcmp. With memcmp, you leak timing information,
> which could then be used to iteratively forge a MAC.
Do you have any pointers where I could learn more about this?
--
Kalle Valo
^ permalink raw reply
* ipmr: MFC routes when VIF deleted
From: Yotam Gigi @ 2017-06-11 8:55 UTC (permalink / raw)
To: netdev@vger.kernel.org; +Cc: Nikolay Aleksandrov, Donald Sharp
I have been looking into some weird behavior, and I am not sure whether it is
a bug or a feature.
When a VIF with index v gets deleted, the MFC routes does not get updated, which
means that there can be routes pointing to that VIF. On datapath, when packet
hits that route, the VIF validity will be checked and will not be sent to that
device (but still, the route does not get updated). Now, if the user creates
another VIF with the same index v but different underlay device, the same route
will forward the traffic to that device.
It is relevant to mention that when user adds a MFC route, only the active VIFs
are used, so the flow of adding a route with dummy VIF indices and then
connecting those VIF indices to real device is not supported. The only way to
create a MFC route that has non existing VIFs is to create one with existing
VIFs and then delete them.
Do we really want to support that? To me, it looks like a buggy flow and I
suggest that upon VIF deletion, the MFC routes will be updated to not point to
any non existing VIF indices.
^ permalink raw reply
* Re: [PATCHv3 net] xfrm: move xfrm_garbage_collect out of xfrm_policy_flush
From: Xin Long @ 2017-06-11 10:51 UTC (permalink / raw)
To: Hangbin Liu; +Cc: network dev, David Miller, Steffen Klassert
In-Reply-To: <1497145460-24614-1-git-send-email-liuhangbin@gmail.com>
On Sun, Jun 11, 2017 at 9:44 AM, Hangbin Liu <liuhangbin@gmail.com> wrote:
> Now we will force to do garbage collection if any policy removed in
> xfrm_policy_flush(). But during xfrm_net_exit(). We call flow_cache_fini()
> first and set set fc->percpu to NULL. Then after we call xfrm_policy_fini()
> -> frxm_policy_flush() -> flow_cache_flush(), we will get NULL pointer
> dereference when check percpu_empty. The code path looks like:
>
> flow_cache_fini()
> - fc->percpu = NULL
> xfrm_policy_fini()
> - xfrm_policy_flush()
> - xfrm_garbage_collect()
> - flow_cache_flush()
> - flow_cache_percpu_empty()
> - fcp = per_cpu_ptr(fc->percpu, cpu)
>
> To reproduce, just add ipsec in netns and then remove the netns.
>
> v2:
> As Xin Long suggested, since only two other places need to call it. move
> xfrm_garbage_collect() outside xfrm_policy_flush().
>
> v3:
> Fix subject mismatch after v2 fix.
>
> Fixes: 35db06912189 ("xfrm: do the garbage collection after flushing policy")
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---
> net/key/af_key.c | 2 ++
> net/xfrm/xfrm_policy.c | 4 ----
> net/xfrm/xfrm_user.c | 1 +
> 3 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/net/key/af_key.c b/net/key/af_key.c
> index 512dc43..5103f92 100644
> --- a/net/key/af_key.c
> +++ b/net/key/af_key.c
> @@ -2755,6 +2755,8 @@ static int pfkey_spdflush(struct sock *sk, struct sk_buff *skb, const struct sad
> int err, err2;
>
> err = xfrm_policy_flush(net, XFRM_POLICY_TYPE_MAIN, true);
> + if (!err)
> + xfrm_garbage_collect(net);
> err2 = unicast_flush_resp(sk, hdr);
> if (err || err2) {
> if (err == -ESRCH) /* empty table - old silent behavior */
> diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
> index ed4e52d..643a18f 100644
> --- a/net/xfrm/xfrm_policy.c
> +++ b/net/xfrm/xfrm_policy.c
> @@ -1006,10 +1006,6 @@ int xfrm_policy_flush(struct net *net, u8 type, bool task_valid)
> err = -ESRCH;
> out:
> spin_unlock_bh(&net->xfrm.xfrm_policy_lock);
> -
> - if (cnt)
> - xfrm_garbage_collect(net);
> -
> return err;
> }
> EXPORT_SYMBOL(xfrm_policy_flush);
> diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
> index 38614df..86116e9 100644
> --- a/net/xfrm/xfrm_user.c
> +++ b/net/xfrm/xfrm_user.c
> @@ -2027,6 +2027,7 @@ static int xfrm_flush_policy(struct sk_buff *skb, struct nlmsghdr *nlh,
> return 0;
> return err;
> }
> + xfrm_garbage_collect(net);
>
> c.data.type = type;
> c.event = nlh->nlmsg_type;
> --
> 2.5.5
>
Reviewed-by: Xin Long <lucien.xin@gmail.com>
though I could still see some typo err in changelog.
^ permalink raw reply
* [PATCH net-next v10 0/4] net sched actions: improve dump performance
From: Jamal Hadi Salim @ 2017-06-11 11:53 UTC (permalink / raw)
To: davem
Cc: netdev, jiri, xiyou.wangcong, eric.dumazet, simon.horman, mrv,
Jamal Hadi Salim
From: Jamal Hadi Salim <jhs@mojatatu.com>
Changes since v9:
-----------------
1) General consensus:
- remove again the use of BIT() to maintain uapi consistency ;->
1) Jiri:
- Add a new netlink type NLA_FLAG_BITS to check for valid bits
and use it instead of inline vetting.
Changes since v8:
-----------------
1) Jiri:
- Add back the use of BIT(). Eventually fix iproute2 instead
- Rename VALID_TCA_FLAGS to VALID_TCA_ROOT_FLAGS
Changes since v7:
-----------------
Jamal:
No changes.
Patch 1 went out twice. Resend without two copies of patch 1
changes since v6:
-----------------
1) DaveM:
New rules for netlink messages. From now on we are going to start
checking for bits that are not used and rejecting anything we dont
understand. In the future this is going to require major changes
to user space code (tc etc). This is just a start.
To quote, David:
"
Again, bits you aren't using now, make sure userspace doesn't
set them. And if it does, reject.
"
Added checks for ensuring things work as above.
2) Jiri:
a)Fix the commit message to properly use "Fixes" description
b)Align assignments for nla_policy
Changes since v5:
----------------
0)
Remove use of BIT() because it is kernel specific. Requires a separate
patch (Jiri can submit that in his cleanups)
1)To paraphrase Eric D.
"memcpy(nla_data(count_attr), &cb->args[1], sizeof(u32));
wont work on 64bit BE machines because cb->args[1]
(which is 64 bit is larger in size than sizeof(u32))"
Fixed
2) Jiri Pirko
i) Spotted a bug fix mixed in the patch for wrong TLV
fix. Add patch 1/3 to address this. Make part of this
series because of dependencies.
ii) Rename ACT_LARGE_DUMP_ON -> TCA_FLAG_LARGE_DUMP_ON
iii) Satisfy Jiri's obsession against the noun "tcaa"
a)Rename struct nlattr *tcaa --> struct nlattr *tb
b)Rename TCAA_ACT_XXX -> TCA_ROOT_XXX
Changes since v4:
-----------------
1) Eric D.
pointed out that when all skb space is used up by the dump
there will be no space to insert the TCAA_ACT_COUNT attribute.
2) Jiri:
i) Change:
enum {
TCAA_UNSPEC,
TCAA_ACT_TAB,
TCAA_ACT_FLAGS,
TCAA_ACT_COUNT,
TCAA_ACT_TIME_FILTER,
__TCAA_MAX
};
#define TCAA_MAX (__TCAA_MAX - 1)
#define ACT_LARGE_DUMP_ON (1 << 0)
to:
enum {
TCAA_UNSPEC,
TCAA_ACT_TAB,
#define TCA_ACT_TAB TCAA_ACT_TAB
TCAA_ACT_FLAGS,
TCAA_ACT_COUNT,
__TCAA_MAX,
#define TCAA_MAX (__TCAA_MAX - 1)
};
#define ACT_LARGE_DUMP_ON BIT(0)
Jiri plans to followup with the rest of the code to make the
style consistent.
ii) Rename attribute TCAA_ACT_TIME_FILTER --> TCAA_ACT_TIME_DELTA
iii) Rename variable jiffy_filter --> jiffy_since
iv) Rename msecs_filter --> msecs_since
v) get rid of unused cb->args[0] and rename cb->args[4] to cb->args[0]
Earlier Changes
----------------
- Jiri mostly on names of things.
Jamal Hadi Salim (4):
net netlink: Add new type NLA_FLAG_BITS
net sched actions: Use proper root attribute table for actions
net sched actions: dump more than TCA_ACT_MAX_PRIO actions per batch
net sched actions: add time filter for action dumping
include/net/netlink.h | 11 +++++++
include/uapi/linux/rtnetlink.h | 40 ++++++++++++++++++++++--
lib/nlattr.c | 25 +++++++++++++++
net/sched/act_api.c | 70 +++++++++++++++++++++++++++++++++++-------
4 files changed, 133 insertions(+), 13 deletions(-)
--
1.9.1
^ permalink raw reply
* [PATCH net-next v10 1/4] net netlink: Add new type NLA_FLAG_BITS
From: Jamal Hadi Salim @ 2017-06-11 11:53 UTC (permalink / raw)
To: davem
Cc: netdev, jiri, xiyou.wangcong, eric.dumazet, simon.horman, mrv,
Jamal Hadi Salim
In-Reply-To: <1497182026-11594-1-git-send-email-jhs@emojatatu.com>
From: Jamal Hadi Salim <jhs@mojatatu.com>
Generic bitflags attribute content sent to the kernel by user.
With this type the user can either set or unset a flag in the
kernel.
The nla_flag_values is a bitmap that defines the values being set
The nla_flag_selector is a bitmask that defines which value is legit.
A check is made to ensure the rules that a kernel subsystem always
conforms to bitflags the kernel already knows about. i.e
if the user tries to set a bit flag that is not understood then
the _it will be rejected_.
In the most basic form, the user specifies the attribute policy as:
[ATTR_GOO] = { .type = NLA_FLAG_BITS, .validation_data = &myvalidflags },
where myvalidflags is the bit mask of the flags the kernel understands.
If the user _does not_ provide myvalidflags then the attribute will
also be rejected.
Examples:
nla_flag_values = 0x0, and nla_flag_selector = 0x1
implies we are selecting bit 1 and we want to set its value to 0.
nla_flag_values = 0x2, and nla_flag_selector = 0x2
implies we are selecting bit 2 and we want to set its value to 1.
This patch also provides an extra feature: a validation callback
that could be speaciliazed for other types.
This feature is intended to be used by a kernel subsystem to check
for a combination of bits being present. Example "bit x is valid
only if bit y and z are present".
So a kernel subsystem could specify validation rules of the following
nature:
[ATTR_GOO] = { .type = MYTYPE,
.validation_data = &myvalidation_data,
.validate_content = mycontent_validator },
With validator callback looking like:
int mycontent_validator(const struct nlattr *nla, void *valid_data)
{
const struct myattribute *user_data = nla_data(nla);
struct myvalidation_struct *valid_data_constraint = valid_data;
... return appropriate error code etc ...
}
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
include/net/netlink.h | 11 +++++++++++
include/uapi/linux/rtnetlink.h | 17 +++++++++++++++++
lib/nlattr.c | 25 +++++++++++++++++++++++++
3 files changed, 53 insertions(+)
diff --git a/include/net/netlink.h b/include/net/netlink.h
index 0170917..8ab9784 100644
--- a/include/net/netlink.h
+++ b/include/net/netlink.h
@@ -6,6 +6,11 @@
#include <linux/jiffies.h>
#include <linux/in6.h>
+struct nla_bit_flags {
+ u32 nla_flag_values;
+ u32 nla_flag_selector;
+};
+
/* ========================================================================
* Netlink Messages and Attributes Interface (As Seen On TV)
* ------------------------------------------------------------------------
@@ -178,6 +183,7 @@ enum {
NLA_S16,
NLA_S32,
NLA_S64,
+ NLA_FLAG_BITS,
__NLA_TYPE_MAX,
};
@@ -206,6 +212,7 @@ enum {
* NLA_MSECS Leaving the length field zero will verify the
* given type fits, using it verifies minimum length
* just like "All other"
+ * NLA_FLAG_BITS A bitmap/bitselector attribute
* All other Minimum length of attribute payload
*
* Example:
@@ -213,11 +220,15 @@ enum {
* [ATTR_FOO] = { .type = NLA_U16 },
* [ATTR_BAR] = { .type = NLA_STRING, .len = BARSIZ },
* [ATTR_BAZ] = { .len = sizeof(struct mystruct) },
+ * [ATTR_GOO] = { .type = NLA_FLAG_BITS, .validation_data = &myvalidflags },
* };
*/
struct nla_policy {
u16 type;
u16 len;
+ void *validation_data;
+ int (*validate_content)(const struct nlattr *nla,
+ const void *validation_data);
};
/**
diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
index 564790e..8f07957 100644
--- a/include/uapi/linux/rtnetlink.h
+++ b/include/uapi/linux/rtnetlink.h
@@ -179,6 +179,23 @@ struct rtattr {
#define RTA_DATA(rta) ((void*)(((char*)(rta)) + RTA_LENGTH(0)))
#define RTA_PAYLOAD(rta) ((int)((rta)->rta_len) - RTA_LENGTH(0))
+/* Generic bitflags attribute content sent to the kernel.
+ *
+ * The nla_flag_values is a bitmap that defines the values being set
+ * The nla_flag_selector is a bitmask that defines which value is legit
+ *
+ * Examples:
+ * nla_flag_values = 0x0, and nla_flag_selector = 0x1
+ * implies we are selecting bit 1 and we want to set its value to 0.
+ *
+ * nla_flag_values = 0x2, and nla_flag_selector = 0x2
+ * implies we are selecting bit 2 and we want to set its value to 1.
+ *
+ */
+struct __nla_bit_flags {
+ __u32 nla_flag_values;
+ __u32 nla_flag_selector;
+};
diff --git a/lib/nlattr.c b/lib/nlattr.c
index a7e0b16..78fed43 100644
--- a/lib/nlattr.c
+++ b/lib/nlattr.c
@@ -27,6 +27,21 @@
[NLA_S64] = sizeof(s64),
};
+static int validate_nla_bit_flags(const struct nlattr *nla, void *valid_data)
+{
+ const struct nla_bit_flags *nbf = nla_data(nla);
+ u32 *valid_flags_mask = valid_data;
+
+ if (!valid_data)
+ return -EINVAL;
+
+
+ if (nbf->nla_flag_values & ~*valid_flags_mask)
+ return -EINVAL;
+
+ return 0;
+}
+
static int validate_nla(const struct nlattr *nla, int maxtype,
const struct nla_policy *policy)
{
@@ -46,6 +61,13 @@ static int validate_nla(const struct nlattr *nla, int maxtype,
return -ERANGE;
break;
+ case NLA_FLAG_BITS:
+ if (attrlen != 8) /* 2 x 32 bits */
+ return -ERANGE;
+
+ return validate_nla_bit_flags(nla, pt->validation_data);
+ break;
+
case NLA_NUL_STRING:
if (pt->len)
minlen = min_t(int, attrlen, pt->len + 1);
@@ -103,6 +125,9 @@ static int validate_nla(const struct nlattr *nla, int maxtype,
return -ERANGE;
}
+ if (pt->validate_content)
+ return pt->validate_content(nla, pt->validation_data);
+
return 0;
}
--
1.9.1
^ permalink raw reply related
* [PATCH net-next v10 2/4] net sched actions: Use proper root attribute table for actions
From: Jamal Hadi Salim @ 2017-06-11 11:53 UTC (permalink / raw)
To: davem
Cc: netdev, jiri, xiyou.wangcong, eric.dumazet, simon.horman, mrv,
Jamal Hadi Salim
In-Reply-To: <1497182026-11594-1-git-send-email-jhs@emojatatu.com>
From: Jamal Hadi Salim <jhs@mojatatu.com>
Bug fix for an issue which has been around for about a decade.
We got away with it because the enumeration was larger than needed.
Fixes: 7ba699c604ab ("[NET_SCHED]: Convert actions from rtnetlink to new netlink API")
Suggested-by: Jiri Pirko <jiri@mellanox.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
net/sched/act_api.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index aed6cf2..400eb6e 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -1072,7 +1072,7 @@ static int tc_ctl_action(struct sk_buff *skb, struct nlmsghdr *n,
struct netlink_ext_ack *extack)
{
struct net *net = sock_net(skb->sk);
- struct nlattr *tca[TCA_ACT_MAX + 1];
+ struct nlattr *tca[TCAA_MAX + 1];
u32 portid = skb ? NETLINK_CB(skb).portid : 0;
int ret = 0, ovr = 0;
@@ -1080,7 +1080,7 @@ static int tc_ctl_action(struct sk_buff *skb, struct nlmsghdr *n,
!netlink_capable(skb, CAP_NET_ADMIN))
return -EPERM;
- ret = nlmsg_parse(n, sizeof(struct tcamsg), tca, TCA_ACT_MAX, NULL,
+ ret = nlmsg_parse(n, sizeof(struct tcamsg), tca, TCAA_MAX, NULL,
extack);
if (ret < 0)
return ret;
--
1.9.1
^ permalink raw reply related
* [PATCH net-next v10 3/4] net sched actions: dump more than TCA_ACT_MAX_PRIO actions per batch
From: Jamal Hadi Salim @ 2017-06-11 11:53 UTC (permalink / raw)
To: davem
Cc: netdev, jiri, xiyou.wangcong, eric.dumazet, simon.horman, mrv,
Jamal Hadi Salim
In-Reply-To: <1497182026-11594-1-git-send-email-jhs@emojatatu.com>
From: Jamal Hadi Salim <jhs@mojatatu.com>
When you dump hundreds of thousands of actions, getting only 32 per
dump batch even when the socket buffer and memory allocations allow
is inefficient.
With this change, the user will get as many as possibly fitting
within the given constraints available to the kernel.
The top level action TLV space is extended. An attribute
TCA_ROOT_FLAGS is used to carry flags; flag TCA_FLAG_LARGE_DUMP_ON
is set by the user indicating the user is capable of processing
these large dumps. Older user space which doesnt set this flag
doesnt get the large (than 32) batches.
The kernel uses the TCA_ROOT_COUNT attribute to tell the user how many
actions are put in a single batch. As such user space app knows how long
to iterate (independent of the type of action being dumped)
instead of hardcoded maximum of 32 thus maintaining backward compat.
Some results dumping 1.5M actions below:
first an unpatched tc which doesnt understand these features...
prompt$ time -p tc actions ls action gact | grep index | wc -l
1500000
real 1388.43
user 2.07
sys 1386.79
Now lets see a patched tc which sets the correct flags when requesting
a dump:
prompt$ time -p updatedtc actions ls action gact | grep index | wc -l
1500000
real 178.13
user 2.02
sys 176.96
That is about 8x performance improvement for tc app which sets its
receive buffer to about 32K.
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
include/uapi/linux/rtnetlink.h | 22 +++++++++++++++++--
net/sched/act_api.c | 50 +++++++++++++++++++++++++++++++++---------
2 files changed, 60 insertions(+), 12 deletions(-)
diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
index 8f07957..7d2030f 100644
--- a/include/uapi/linux/rtnetlink.h
+++ b/include/uapi/linux/rtnetlink.h
@@ -693,10 +693,28 @@ struct tcamsg {
unsigned char tca__pad1;
unsigned short tca__pad2;
};
+
+enum {
+ TCA_ROOT_UNSPEC,
+ TCA_ROOT_TAB,
+#define TCA_ACT_TAB TCA_ROOT_TAB
+#define TCAA_MAX TCA_ROOT_TAB
+ TCA_ROOT_FLAGS,
+ TCA_ROOT_COUNT,
+ __TCA_ROOT_MAX,
+#define TCA_ROOT_MAX (__TCA_ROOT_MAX - 1)
+};
+
#define TA_RTA(r) ((struct rtattr*)(((char*)(r)) + NLMSG_ALIGN(sizeof(struct tcamsg))))
#define TA_PAYLOAD(n) NLMSG_PAYLOAD(n,sizeof(struct tcamsg))
-#define TCA_ACT_TAB 1 /* attr type must be >=1 */
-#define TCAA_MAX 1
+/* tcamsg flags stored in attribute TCA_ROOT_FLAGS
+ *
+ * TCA_FLAG_LARGE_DUMP_ON user->kernel to request for larger than TCA_ACT_MAX_PRIO
+ * actions in a dump. All dump responses will contain the number of actions
+ * being dumped stored in for user app's consumption in TCA_ROOT_COUNT
+ *
+ */
+#define TCA_FLAG_LARGE_DUMP_ON (1 << 0)
/* New extended info filters for IFLA_EXT_MASK */
#define RTEXT_FILTER_VF (1 << 0)
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 400eb6e..935dc19 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -110,6 +110,7 @@ static int tcf_dump_walker(struct tcf_hashinfo *hinfo, struct sk_buff *skb,
struct netlink_callback *cb)
{
int err = 0, index = -1, i = 0, s_i = 0, n_i = 0;
+ u32 act_flags = cb->args[2];
struct nlattr *nest;
spin_lock_bh(&hinfo->lock);
@@ -138,14 +139,18 @@ static int tcf_dump_walker(struct tcf_hashinfo *hinfo, struct sk_buff *skb,
}
nla_nest_end(skb, nest);
n_i++;
- if (n_i >= TCA_ACT_MAX_PRIO)
+ if (!(act_flags & TCA_FLAG_LARGE_DUMP_ON) &&
+ n_i >= TCA_ACT_MAX_PRIO)
goto done;
}
}
done:
spin_unlock_bh(&hinfo->lock);
- if (n_i)
+ if (n_i) {
cb->args[0] += n_i;
+ if (act_flags & TCA_FLAG_LARGE_DUMP_ON)
+ cb->args[1] = n_i;
+ }
return n_i;
nla_put_failure:
@@ -1068,11 +1073,17 @@ static int tcf_action_add(struct net *net, struct nlattr *nla,
return tcf_add_notify(net, n, &actions, portid);
}
+static u32 allowed_flags = TCA_FLAG_LARGE_DUMP_ON;
+static const struct nla_policy tcaa_policy[TCA_ROOT_MAX + 1] = {
+ [TCA_ROOT_FLAGS] = { .type = NLA_FLAG_BITS,
+ .validation_data = &allowed_flags },
+};
+
static int tc_ctl_action(struct sk_buff *skb, struct nlmsghdr *n,
struct netlink_ext_ack *extack)
{
struct net *net = sock_net(skb->sk);
- struct nlattr *tca[TCAA_MAX + 1];
+ struct nlattr *tca[TCA_ROOT_MAX + 1];
u32 portid = skb ? NETLINK_CB(skb).portid : 0;
int ret = 0, ovr = 0;
@@ -1080,7 +1091,7 @@ static int tc_ctl_action(struct sk_buff *skb, struct nlmsghdr *n,
!netlink_capable(skb, CAP_NET_ADMIN))
return -EPERM;
- ret = nlmsg_parse(n, sizeof(struct tcamsg), tca, TCAA_MAX, NULL,
+ ret = nlmsg_parse(n, sizeof(struct tcamsg), tca, TCA_ROOT_MAX, NULL,
extack);
if (ret < 0)
return ret;
@@ -1121,16 +1132,12 @@ static int tc_ctl_action(struct sk_buff *skb, struct nlmsghdr *n,
return ret;
}
-static struct nlattr *find_dump_kind(const struct nlmsghdr *n)
+static struct nlattr *find_dump_kind(struct nlattr **nla)
{
struct nlattr *tb1, *tb2[TCA_ACT_MAX + 1];
struct nlattr *tb[TCA_ACT_MAX_PRIO + 1];
- struct nlattr *nla[TCAA_MAX + 1];
struct nlattr *kind;
- if (nlmsg_parse(n, sizeof(struct tcamsg), nla, TCAA_MAX,
- NULL, NULL) < 0)
- return NULL;
tb1 = nla[TCA_ACT_TAB];
if (tb1 == NULL)
return NULL;
@@ -1157,8 +1164,18 @@ static int tc_dump_action(struct sk_buff *skb, struct netlink_callback *cb)
struct tc_action_ops *a_o;
int ret = 0;
struct tcamsg *t = (struct tcamsg *) nlmsg_data(cb->nlh);
- struct nlattr *kind = find_dump_kind(cb->nlh);
+ struct __nla_bit_flags select_flags;
+ struct nlattr *count_attr = NULL;
+ struct nlattr *tb[TCA_ROOT_MAX + 1];
+ struct nlattr *kind = NULL;
+ u32 act_count = 0;
+
+ ret = nlmsg_parse(cb->nlh, sizeof(struct tcamsg), tb, TCA_ROOT_MAX,
+ tcaa_policy, NULL);
+ if (ret < 0)
+ return ret;
+ kind = find_dump_kind(tb);
if (kind == NULL) {
pr_info("tc_dump_action: action bad kind\n");
return 0;
@@ -1168,14 +1185,24 @@ static int tc_dump_action(struct sk_buff *skb, struct netlink_callback *cb)
if (a_o == NULL)
return 0;
+ if (tb[TCA_ROOT_FLAGS])
+ nla_memcpy(&select_flags, tb[TCA_ROOT_FLAGS],
+ sizeof(select_flags));
+
nlh = nlmsg_put(skb, NETLINK_CB(cb->skb).portid, cb->nlh->nlmsg_seq,
cb->nlh->nlmsg_type, sizeof(*t), 0);
if (!nlh)
goto out_module_put;
+
+ cb->args[2] = select_flags.nla_flag_values;
t = nlmsg_data(nlh);
t->tca_family = AF_UNSPEC;
t->tca__pad1 = 0;
t->tca__pad2 = 0;
+ count_attr = nla_reserve(skb, TCA_ROOT_COUNT, sizeof(u32));
+ if (!count_attr)
+ goto out_module_put;
+
nest = nla_nest_start(skb, TCA_ACT_TAB);
if (nest == NULL)
@@ -1188,6 +1215,9 @@ static int tc_dump_action(struct sk_buff *skb, struct netlink_callback *cb)
if (ret > 0) {
nla_nest_end(skb, nest);
ret = skb->len;
+ act_count = cb->args[1];
+ memcpy(nla_data(count_attr), &act_count, sizeof(u32));
+ cb->args[1] = 0;
} else
nlmsg_trim(skb, b);
--
1.9.1
^ permalink raw reply related
* [PATCH net-next v10 4/4] net sched actions: add time filter for action dumping
From: Jamal Hadi Salim @ 2017-06-11 11:53 UTC (permalink / raw)
To: davem
Cc: netdev, jiri, xiyou.wangcong, eric.dumazet, simon.horman, mrv,
Jamal Hadi Salim
In-Reply-To: <1497182026-11594-1-git-send-email-jhs@emojatatu.com>
From: Jamal Hadi Salim <jhs@mojatatu.com>
This adds support for filtering based on time since last used.
When we are dumping a large number of actions it is useful to
have the option of filtering based on when the action was last
used to reduce the amount of data crossing to user space.
With this patch the user space app sets the TCA_ROOT_TIME_DELTA
attribute with the value in milliseconds with "time of interest
since now". The kernel converts this to jiffies and does the
filtering comparison matching entries that have seen activity
since then and returns them to user space.
Old kernels and old tc continue to work in legacy mode since
they dont specify this attribute.
Some example (we have 400 actions bound to 400 filters); at
installation time using hacked tc which sets the time of
interest to 120 seconds:
prompt$ hackedtc actions ls action gact | grep index | wc -l
400
go get some coffee and wait for > 120 seconds and try again:
prompt$ hackedtc actions ls action gact | grep index | wc -l
0
Lets see a filter bound to one of these actions:
..
filter pref 10 u32
filter pref 10 u32 fh 800: ht divisor 1
filter pref 10 u32 fh 800::800 order 2048 key ht 800 bkt 0 flowid 1:10 (rule hit 2 success 1)
match 7f000002/ffffffff at 12 (success 1 )
action order 1: gact action pass
random type none pass val 0
index 23 ref 2 bind 1 installed 1145 sec used 802 sec
Action statistics:
Sent 84 bytes 1 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
....
that coffee took long, no? It was good.
Now lets ping -c 1 127.0.0.2, then run the actions again:
prompt$ hackedtc actions ls action gact | grep index | wc -l
1
More details please:
prompt$ hackedtc -s actions ls action gact
action order 0: gact action pass
random type none pass val 0
index 23 ref 2 bind 1 installed 1270 sec used 30 sec
Action statistics:
Sent 168 bytes 2 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
And the filter?
filter pref 10 u32
filter pref 10 u32 fh 800: ht divisor 1
filter pref 10 u32 fh 800::800 order 2048 key ht 800 bkt 0 flowid 1:10 (rule hit 4 success 2)
match 7f000002/ffffffff at 12 (success 2 )
action order 1: gact action pass
random type none pass val 0
index 23 ref 2 bind 1 installed 1324 sec used 84 sec
Action statistics:
Sent 168 bytes 2 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
include/uapi/linux/rtnetlink.h | 1 +
net/sched/act_api.c | 20 +++++++++++++++++++-
2 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
index 7d2030f..988ebc2 100644
--- a/include/uapi/linux/rtnetlink.h
+++ b/include/uapi/linux/rtnetlink.h
@@ -701,6 +701,7 @@ enum {
#define TCAA_MAX TCA_ROOT_TAB
TCA_ROOT_FLAGS,
TCA_ROOT_COUNT,
+ TCA_ROOT_TIME_DELTA, /* in msecs */
__TCA_ROOT_MAX,
#define TCA_ROOT_MAX (__TCA_ROOT_MAX - 1)
};
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 935dc19..aa9549f 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -111,6 +111,7 @@ static int tcf_dump_walker(struct tcf_hashinfo *hinfo, struct sk_buff *skb,
{
int err = 0, index = -1, i = 0, s_i = 0, n_i = 0;
u32 act_flags = cb->args[2];
+ unsigned long jiffy_since = cb->args[3];
struct nlattr *nest;
spin_lock_bh(&hinfo->lock);
@@ -128,6 +129,11 @@ static int tcf_dump_walker(struct tcf_hashinfo *hinfo, struct sk_buff *skb,
if (index < s_i)
continue;
+ if (jiffy_since &&
+ time_after(jiffy_since,
+ (unsigned long)p->tcfa_tm.lastuse))
+ continue;
+
nest = nla_nest_start(skb, n_i);
if (nest == NULL)
goto nla_put_failure;
@@ -145,9 +151,11 @@ static int tcf_dump_walker(struct tcf_hashinfo *hinfo, struct sk_buff *skb,
}
}
done:
+ if (index >= 0)
+ cb->args[0] = index + 1;
+
spin_unlock_bh(&hinfo->lock);
if (n_i) {
- cb->args[0] += n_i;
if (act_flags & TCA_FLAG_LARGE_DUMP_ON)
cb->args[1] = n_i;
}
@@ -1077,6 +1085,7 @@ static int tcf_action_add(struct net *net, struct nlattr *nla,
static const struct nla_policy tcaa_policy[TCA_ROOT_MAX + 1] = {
[TCA_ROOT_FLAGS] = { .type = NLA_FLAG_BITS,
.validation_data = &allowed_flags },
+ [TCA_ROOT_TIME_DELTA] = { .type = NLA_U32 },
};
static int tc_ctl_action(struct sk_buff *skb, struct nlmsghdr *n,
@@ -1167,7 +1176,9 @@ static int tc_dump_action(struct sk_buff *skb, struct netlink_callback *cb)
struct __nla_bit_flags select_flags;
struct nlattr *count_attr = NULL;
struct nlattr *tb[TCA_ROOT_MAX + 1];
+ unsigned long jiffy_since = 0;
struct nlattr *kind = NULL;
+ u32 msecs_since = 0;
u32 act_count = 0;
ret = nlmsg_parse(cb->nlh, sizeof(struct tcamsg), tb, TCA_ROOT_MAX,
@@ -1189,12 +1200,19 @@ static int tc_dump_action(struct sk_buff *skb, struct netlink_callback *cb)
nla_memcpy(&select_flags, tb[TCA_ROOT_FLAGS],
sizeof(select_flags));
+ if (tb[TCA_ROOT_TIME_DELTA])
+ msecs_since = nla_get_u32(tb[TCA_ROOT_TIME_DELTA]);
+
nlh = nlmsg_put(skb, NETLINK_CB(cb->skb).portid, cb->nlh->nlmsg_seq,
cb->nlh->nlmsg_type, sizeof(*t), 0);
if (!nlh)
goto out_module_put;
+ if (msecs_since)
+ jiffy_since = jiffies - msecs_to_jiffies(msecs_since);
+
cb->args[2] = select_flags.nla_flag_values;
+ cb->args[3] = jiffy_since;
t = nlmsg_data(nlh);
t->tca_family = AF_UNSPEC;
t->tca__pad1 = 0;
--
1.9.1
^ permalink raw reply related
* Re: [PATCH net-next 1/1] net: reflect mark on tcp syn ack packets
From: Jamal Hadi Salim @ 2017-06-11 11:58 UTC (permalink / raw)
To: davem; +Cc: netdev, lorenzo, mrv, hadi
In-Reply-To: <20170610.190213.1437094451353913622.davem@davemloft.net>
On 17-06-10 07:02 PM, David Miller wrote:
>> @@ -173,7 +173,8 @@ int ip_build_and_send_pkt(struct sk_buff *skb, const struct sock *sk,
>> }
>>
>> skb->priority = sk->sk_priority;
>> - skb->mark = sk->sk_mark;
>> + if (!skb->mark)
>> + skb->mark = sk->sk_mark;
>
> Maybe this should both be "inet_request_mark()"?
>
Challenge is making of a synack requires a new allocated skb;
and sk is a listening socket - which should/has a mark of
0 meaning at ip_build_and_send_pkt() it overrides
the value already set on the skb->mark.
cheers,
jamal
> Also, Lorenzo, please review.
>
^ permalink raw reply
* Re: [PATCH net-next v10 0/4] net sched actions: improve dump performance
From: Jamal Hadi Salim @ 2017-06-11 12:06 UTC (permalink / raw)
To: davem; +Cc: netdev, jiri, xiyou.wangcong, eric.dumazet, simon.horman, mrv
In-Reply-To: <1497182026-11594-1-git-send-email-jhs@emojatatu.com>
[-- Attachment #1: Type: text/plain, Size: 68 bytes --]
Attached iproute2 used for testing these features..
cheers,
jamal
[-- Attachment #2: patch-large-dump-iproute2 --]
[-- Type: text/plain, Size: 13447 bytes --]
diff --git a/Makefile b/Makefile
index 18de7dc..ed3c10a 100644
--- a/Makefile
+++ b/Makefile
@@ -45,7 +45,7 @@ HOSTCC ?= $(CC)
DEFINES += -D_GNU_SOURCE
# Turn on transparent support for LFS
DEFINES += -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE
-CCOPTS = -O2
+CCOPTS = -g
WFLAGS := -Wall -Wstrict-prototypes -Wmissing-prototypes
WFLAGS += -Wmissing-declarations -Wold-style-definition -Wformat=2
diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
index a96db83..988ebc2 100644
--- a/include/linux/rtnetlink.h
+++ b/include/linux/rtnetlink.h
@@ -1,5 +1,5 @@
-#ifndef __LINUX_RTNETLINK_H
-#define __LINUX_RTNETLINK_H
+#ifndef _UAPI__LINUX_RTNETLINK_H
+#define _UAPI__LINUX_RTNETLINK_H
#include <linux/types.h>
#include <linux/netlink.h>
@@ -179,6 +179,23 @@ struct rtattr {
#define RTA_DATA(rta) ((void*)(((char*)(rta)) + RTA_LENGTH(0)))
#define RTA_PAYLOAD(rta) ((int)((rta)->rta_len) - RTA_LENGTH(0))
+/* Generic bitflags attribute content sent to the kernel.
+ *
+ * The nla_flag_values is a bitmap that defines the values being set
+ * The nla_flag_selector is a bitmask that defines which value is legit
+ *
+ * Examples:
+ * nla_flag_values = 0x0, and nla_flag_selector = 0x1
+ * implies we are selecting bit 1 and we want to set its value to 0.
+ *
+ * nla_flag_values = 0x2, and nla_flag_selector = 0x2
+ * implies we are selecting bit 2 and we want to set its value to 1.
+ *
+ */
+struct __nla_bit_flags {
+ __u32 nla_flag_values;
+ __u32 nla_flag_selector;
+};
@@ -278,6 +295,7 @@ enum rt_scope_t {
#define RTM_F_EQUALIZE 0x400 /* Multipath equalizer: NI */
#define RTM_F_PREFIX 0x800 /* Prefix addresses */
#define RTM_F_LOOKUP_TABLE 0x1000 /* set rtm_table to FIB lookup result */
+#define RTM_F_FIB_MATCH 0x2000 /* return full fib lookup match */
/* Reserved table identifiers */
@@ -549,6 +567,7 @@ enum {
TCA_STAB,
TCA_PAD,
TCA_DUMP_INVISIBLE,
+ TCA_CHAIN,
__TCA_MAX
};
@@ -581,6 +600,7 @@ enum {
#define NDUSEROPT_MAX (__NDUSEROPT_MAX - 1)
+#ifndef __KERNEL__
/* RTnetlink multicast groups - backwards compatibility for userspace */
#define RTMGRP_LINK 1
#define RTMGRP_NOTIFY 2
@@ -601,6 +621,7 @@ enum {
#define RTMGRP_DECnet_ROUTE 0x4000
#define RTMGRP_IPV6_PREFIX 0x20000
+#endif
/* RTnetlink multicast groups */
enum rtnetlink_groups {
@@ -672,10 +693,29 @@ struct tcamsg {
unsigned char tca__pad1;
unsigned short tca__pad2;
};
+
+enum {
+ TCA_ROOT_UNSPEC,
+ TCA_ROOT_TAB,
+#define TCA_ACT_TAB TCA_ROOT_TAB
+#define TCAA_MAX TCA_ROOT_TAB
+ TCA_ROOT_FLAGS,
+ TCA_ROOT_COUNT,
+ TCA_ROOT_TIME_DELTA, /* in msecs */
+ __TCA_ROOT_MAX,
+#define TCA_ROOT_MAX (__TCA_ROOT_MAX - 1)
+};
+
#define TA_RTA(r) ((struct rtattr*)(((char*)(r)) + NLMSG_ALIGN(sizeof(struct tcamsg))))
#define TA_PAYLOAD(n) NLMSG_PAYLOAD(n,sizeof(struct tcamsg))
-#define TCA_ACT_TAB 1 /* attr type must be >=1 */
-#define TCAA_MAX 1
+/* tcamsg flags stored in attribute TCA_ROOT_FLAGS
+ *
+ * TCA_FLAG_LARGE_DUMP_ON user->kernel to request for larger than TCA_ACT_MAX_PRIO
+ * actions in a dump. All dump responses will contain the number of actions
+ * being dumped stored in for user app's consumption in TCA_ROOT_COUNT
+ *
+ */
+#define TCA_FLAG_LARGE_DUMP_ON (1 << 0)
/* New extended info filters for IFLA_EXT_MASK */
#define RTEXT_FILTER_VF (1 << 0)
@@ -687,4 +727,4 @@ struct tcamsg {
-#endif /* __LINUX_RTNETLINK_H */
+#endif /* _UAPI__LINUX_RTNETLINK_H */
diff --git a/tc/f_basic.c b/tc/f_basic.c
index d663668..8370ea6 100644
--- a/tc/f_basic.c
+++ b/tc/f_basic.c
@@ -135,7 +135,7 @@ static int basic_print_opt(struct filter_util *qu, FILE *f,
}
if (tb[TCA_BASIC_ACT]) {
- tc_print_action(f, tb[TCA_BASIC_ACT]);
+ tc_print_action(f, tb[TCA_BASIC_ACT], 0);
}
return 0;
diff --git a/tc/f_bpf.c b/tc/f_bpf.c
index 75c44c0..6581293 100644
--- a/tc/f_bpf.c
+++ b/tc/f_bpf.c
@@ -236,7 +236,7 @@ static int bpf_print_opt(struct filter_util *qu, FILE *f,
}
if (tb[TCA_BPF_ACT])
- tc_print_action(f, tb[TCA_BPF_ACT]);
+ tc_print_action(f, tb[TCA_BPF_ACT], 0);
return 0;
}
diff --git a/tc/f_cgroup.c b/tc/f_cgroup.c
index ecf9909..633700e 100644
--- a/tc/f_cgroup.c
+++ b/tc/f_cgroup.c
@@ -102,7 +102,7 @@ static int cgroup_print_opt(struct filter_util *qu, FILE *f,
}
if (tb[TCA_CGROUP_ACT])
- tc_print_action(f, tb[TCA_CGROUP_ACT]);
+ tc_print_action(f, tb[TCA_CGROUP_ACT], 0);
return 0;
}
diff --git a/tc/f_flow.c b/tc/f_flow.c
index 09ddcaa..b157104 100644
--- a/tc/f_flow.c
+++ b/tc/f_flow.c
@@ -347,7 +347,7 @@ static int flow_print_opt(struct filter_util *fu, FILE *f, struct rtattr *opt,
tc_print_police(f, tb[TCA_FLOW_POLICE]);
if (tb[TCA_FLOW_ACT]) {
fprintf(f, "\n");
- tc_print_action(f, tb[TCA_FLOW_ACT]);
+ tc_print_action(f, tb[TCA_FLOW_ACT], 0);
}
return 0;
}
diff --git a/tc/f_flower.c b/tc/f_flower.c
index ebc63ca..d015ade 100644
--- a/tc/f_flower.c
+++ b/tc/f_flower.c
@@ -1179,7 +1179,7 @@ static int flower_print_opt(struct filter_util *qu, FILE *f,
}
if (tb[TCA_FLOWER_ACT])
- tc_print_action(f, tb[TCA_FLOWER_ACT]);
+ tc_print_action(f, tb[TCA_FLOWER_ACT], 0);
return 0;
}
diff --git a/tc/f_fw.c b/tc/f_fw.c
index 790bef9..c39789b 100644
--- a/tc/f_fw.c
+++ b/tc/f_fw.c
@@ -160,7 +160,7 @@ static int fw_print_opt(struct filter_util *qu, FILE *f, struct rtattr *opt, __u
if (tb[TCA_FW_ACT]) {
fprintf(f, "\n");
- tc_print_action(f, tb[TCA_FW_ACT]);
+ tc_print_action(f, tb[TCA_FW_ACT], 0);
}
return 0;
}
diff --git a/tc/f_matchall.c b/tc/f_matchall.c
index 5a51e75..d78660e 100644
--- a/tc/f_matchall.c
+++ b/tc/f_matchall.c
@@ -145,7 +145,7 @@ static int matchall_print_opt(struct filter_util *qu, FILE *f,
}
if (tb[TCA_MATCHALL_ACT])
- tc_print_action(f, tb[TCA_MATCHALL_ACT]);
+ tc_print_action(f, tb[TCA_MATCHALL_ACT], 0);
return 0;
}
diff --git a/tc/f_route.c b/tc/f_route.c
index 30514c4..e88313f 100644
--- a/tc/f_route.c
+++ b/tc/f_route.c
@@ -168,7 +168,7 @@ static int route_print_opt(struct filter_util *qu, FILE *f, struct rtattr *opt,
if (tb[TCA_ROUTE4_POLICE])
tc_print_police(f, tb[TCA_ROUTE4_POLICE]);
if (tb[TCA_ROUTE4_ACT])
- tc_print_action(f, tb[TCA_ROUTE4_ACT]);
+ tc_print_action(f, tb[TCA_ROUTE4_ACT], 0);
return 0;
}
diff --git a/tc/f_rsvp.c b/tc/f_rsvp.c
index 94bfbef..65caeb4 100644
--- a/tc/f_rsvp.c
+++ b/tc/f_rsvp.c
@@ -402,7 +402,7 @@ static int rsvp_print_opt(struct filter_util *qu, FILE *f, struct rtattr *opt, _
}
if (tb[TCA_RSVP_ACT]) {
- tc_print_action(f, tb[TCA_RSVP_ACT]);
+ tc_print_action(f, tb[TCA_RSVP_ACT], 0);
}
if (tb[TCA_RSVP_POLICE])
tc_print_police(f, tb[TCA_RSVP_POLICE]);
diff --git a/tc/f_tcindex.c b/tc/f_tcindex.c
index 784c890..dd1cb47 100644
--- a/tc/f_tcindex.c
+++ b/tc/f_tcindex.c
@@ -173,7 +173,7 @@ static int tcindex_print_opt(struct filter_util *qu, FILE *f,
}
if (tb[TCA_TCINDEX_ACT]) {
fprintf(f, "\n");
- tc_print_action(f, tb[TCA_TCINDEX_ACT]);
+ tc_print_action(f, tb[TCA_TCINDEX_ACT], 0);
}
return 0;
}
diff --git a/tc/f_u32.c b/tc/f_u32.c
index ff700e9..4abb2fa 100644
--- a/tc/f_u32.c
+++ b/tc/f_u32.c
@@ -1337,7 +1337,7 @@ static int u32_print_opt(struct filter_util *qu, FILE *f, struct rtattr *opt,
}
if (tb[TCA_U32_ACT])
- tc_print_action(f, tb[TCA_U32_ACT]);
+ tc_print_action(f, tb[TCA_U32_ACT], 0);
return 0;
}
diff --git a/tc/m_action.c b/tc/m_action.c
index 6ebe85e..88a377c 100644
--- a/tc/m_action.c
+++ b/tc/m_action.c
@@ -346,21 +346,24 @@ tc_print_action_flush(FILE *f, const struct rtattr *arg)
}
int
-tc_print_action(FILE *f, const struct rtattr *arg)
+tc_print_action(FILE *f, const struct rtattr *arg, unsigned short tot_acts)
{
int i;
- struct rtattr *tb[TCA_ACT_MAX_PRIO + 1];
if (arg == NULL)
return 0;
- parse_rtattr_nested(tb, TCA_ACT_MAX_PRIO, arg);
+ if (!tot_acts)
+ tot_acts = TCA_ACT_MAX_PRIO;
+
+ struct rtattr *tb[tot_acts + 1];
+ parse_rtattr_nested(tb, tot_acts, arg);
if (tab_flush && NULL != tb[0] && NULL == tb[1])
return tc_print_action_flush(f, tb[0]);
- for (i = 0; i < TCA_ACT_MAX_PRIO; i++) {
+ for (i = 0; i < tot_acts; i++) {
if (tb[i]) {
fprintf(f, "\n\taction order %d: ", i);
if (tc_print_one_action(f, tb[i]) < 0) {
@@ -380,7 +383,8 @@ int print_action(const struct sockaddr_nl *who,
FILE *fp = (FILE *)arg;
struct tcamsg *t = NLMSG_DATA(n);
int len = n->nlmsg_len;
- struct rtattr *tb[TCAA_MAX+1];
+ __u32 *tot_acts = NULL;
+ struct rtattr *tb[TCA_ROOT_MAX+1];
len -= NLMSG_LENGTH(sizeof(*t));
@@ -389,8 +393,12 @@ int print_action(const struct sockaddr_nl *who,
return -1;
}
- parse_rtattr(tb, TCAA_MAX, TA_RTA(t), len);
+ parse_rtattr(tb, TCA_ROOT_MAX, TA_RTA(t), len);
+
+ if (tb[TCA_ROOT_COUNT])
+ tot_acts = RTA_DATA(tb[TCA_ROOT_COUNT]);
+ fprintf(fp, "total acts %d \n", tot_acts?*tot_acts:0);
if (tb[TCA_ACT_TAB] == NULL) {
if (n->nlmsg_type != RTM_GETACTION)
fprintf(stderr, "print_action: NULL kind\n");
@@ -414,7 +422,9 @@ int print_action(const struct sockaddr_nl *who,
fprintf(fp, "Replaced action ");
}
}
- tc_print_action(fp, tb[TCA_ACT_TAB]);
+
+
+ tc_print_action(fp, tb[TCA_ACT_TAB], tot_acts?*tot_acts:0);
return 0;
}
@@ -427,7 +437,7 @@ static int tc_action_gd(int cmd, unsigned int flags, int *argc_p, char ***argv_p
char **argv = *argv_p;
int prio = 0;
int ret = 0;
- __u32 i;
+ __u32 i = 0;
struct rtattr *tail;
struct rtattr *tail2;
struct nlmsghdr *ans = NULL;
@@ -498,7 +508,8 @@ static int tc_action_gd(int cmd, unsigned int flags, int *argc_p, char ***argv_p
tail2 = NLMSG_TAIL(&req.n);
addattr_l(&req.n, MAX_MSG, ++prio, NULL, 0);
addattr_l(&req.n, MAX_MSG, TCA_ACT_KIND, k, strlen(k) + 1);
- addattr32(&req.n, MAX_MSG, TCA_ACT_INDEX, i);
+ if (i > 0)
+ addattr32(&req.n, MAX_MSG, TCA_ACT_INDEX, i);
tail2->rta_len = (void *) NLMSG_TAIL(&req.n) - (void *) tail2;
}
@@ -561,12 +572,16 @@ static int tc_action_modify(int cmd, unsigned int flags, int *argc_p, char ***ar
return ret;
}
-static int tc_act_list_or_flush(int argc, char **argv, int event)
+static int tc_act_list_or_flush(int *argc_p, char ***argv_p, int event)
{
+ struct rtattr *tail, *tail2, *tail3, *tail4;
int ret = 0, prio = 0, msg_size = 0;
- char k[16];
- struct rtattr *tail, *tail2;
struct action_util *a = NULL;
+ struct __nla_bit_flags flag_select = { 0 };
+ char **argv = *argv_p;
+ __u32 msec_since = 0;
+ int argc = *argc_p;
+ char k[16];
struct {
struct nlmsghdr n;
struct tcamsg t;
@@ -597,11 +612,38 @@ static int tc_act_list_or_flush(int argc, char **argv, int event)
}
strncpy(k, *argv, sizeof(k) - 1);
+ argc -= 1;
+ argv += 1;
+
+ fprintf(stderr, "%d: <%s>\n", argc, *argv);
+
+ if (argc && (strcmp(*argv, "since") == 0)) {
+ NEXT_ARG();
+ if (get_u32(&msec_since, *argv, 0))
+ invarg("dump time \"since\" is invalid", *argv);
+ }
+
addattr_l(&req.n, MAX_MSG, ++prio, NULL, 0);
addattr_l(&req.n, MAX_MSG, TCA_ACT_KIND, k, strlen(k) + 1);
tail2->rta_len = (void *) NLMSG_TAIL(&req.n) - (void *) tail2;
tail->rta_len = (void *) NLMSG_TAIL(&req.n) - (void *) tail;
+ tail3 = NLMSG_TAIL(&req.n);
+ flag_select.nla_flag_values |= TCA_FLAG_LARGE_DUMP_ON;
+ flag_select.nla_flag_selector |= TCA_FLAG_LARGE_DUMP_ON;
+#if 0
+ flag_select.nla_flag_values |= 8; /* test rejection */
+ flag_select.nla_flag_selector |= 8; /* test rejection */
+#endif
+ addattr_l(&req.n, MAX_MSG, TCA_ROOT_FLAGS, &flag_select,
+ sizeof(struct __nla_bit_flags));
+ tail3->rta_len = (void *) NLMSG_TAIL(&req.n) - (void *) tail3;
+ if (msec_since) {
+ fprintf(stderr, "XXX: since %d\n", msec_since);
+ tail4 = NLMSG_TAIL(&req.n);
+ addattr32(&req.n, MAX_MSG, TCA_ROOT_TIME_DELTA, msec_since);
+ tail4->rta_len = (void *) NLMSG_TAIL(&req.n) - (void *) tail4;
+ }
msg_size = NLMSG_ALIGN(req.n.nlmsg_len) - NLMSG_ALIGN(sizeof(struct nlmsghdr));
if (event == RTM_GETACTION) {
@@ -626,6 +668,8 @@ static int tc_act_list_or_flush(int argc, char **argv, int event)
bad_val:
+ *argc_p = argc;
+ *argv_p = argv;
return ret;
}
@@ -655,13 +699,21 @@ int do_action(int argc, char **argv)
act_usage();
return -1;
}
- return tc_act_list_or_flush(argc-2, argv+2, RTM_GETACTION);
+
+ argc -= 2;
+ argv += 2;
+ return tc_act_list_or_flush(&argc, &argv,
+ RTM_GETACTION);
} else if (matches(*argv, "flush") == 0) {
if (argc <= 2) {
act_usage();
return -1;
}
- return tc_act_list_or_flush(argc-2, argv+2, RTM_DELACTION);
+
+ argc -= 2;
+ argv += 2;
+ return tc_act_list_or_flush(&argc, &argv,
+ RTM_DELACTION);
} else if (matches(*argv, "help") == 0) {
act_usage();
return -1;
diff --git a/tc/tc_util.h b/tc/tc_util.h
index 4db26c6..af578f9 100644
--- a/tc/tc_util.h
+++ b/tc/tc_util.h
@@ -106,7 +106,7 @@ int act_parse_police(struct action_util *a, int *argc_p,
char ***argv_p, int tca_id, struct nlmsghdr *n);
int print_police(struct action_util *a, FILE *f, struct rtattr *tb);
int police_print_xstats(struct action_util *a, FILE *f, struct rtattr *tb);
-int tc_print_action(FILE *f, const struct rtattr *tb);
+int tc_print_action(FILE *f, const struct rtattr *tb, unsigned short tot_acts);
int tc_print_ipt(FILE *f, const struct rtattr *tb);
int parse_action(int *argc_p, char ***argv_p, int tca_id, struct nlmsghdr *n);
void print_tm(FILE *f, const struct tcf_t *tm);
^ permalink raw reply related
* Re: [PATCH net-next 0/8] Bug fixes in ena ethernet driver
From: Belgazal, Netanel @ 2017-06-11 12:28 UTC (permalink / raw)
To: David Miller, Florian Fainelli
Cc: netdev@vger.kernel.org, Woodhouse, David, Machulsky, Zorik,
Matushevsky, Alexander, BSHARA, Said, Wilson, Matt,
Liguori, Anthony, Bshara, Nafea, Schmeilin, Evgeny
In-Reply-To: <20170610.161133.722413665016881379.davem@davemloft.net>
Ack.
resubmitting to net branch and adding "Fixes" mark.
________________________________________
From: David Miller <davem@davemloft.net>
Sent: Saturday, June 10, 2017 11:11 PM
To: f.fainelli@gmail.com
Cc: Belgazal, Netanel; netdev@vger.kernel.org; Woodhouse, David; Machulsky, Zorik; Matushevsky, Alexander; BSHARA, Said; Wilson, Matt; Liguori, Anthony; Bshara, Nafea; Schmeilin, Evgeny
Subject: Re: [PATCH net-next 0/8] Bug fixes in ena ethernet driver
From: Florian Fainelli <f.fainelli@gmail.com>
Date: Fri, 9 Jun 2017 15:19:54 -0700
> On 06/09/2017 03:13 PM, netanel@amazon.com wrote:
>> From: Netanel Belgazal <netanel@amazon.com>
>>
>> This patchset contains fixes for the bugs that were discovered so far.
>
> If these are all fixes you should submit them against the "net" tree.
> net-next is for features [1].
>
> Since these are fixes, you may also want to provide a Fixes: 12-digit
> commit ("commit subject") [2] such that David can queue these patches
> for stable trees and this can be retrofitted into kernel distributions.
>
> [1]:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/networking/netdev-FAQ.txt#n25
>
> [2]:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst#n183
Yeah I agree. If they are genuine bug fixes they should be submitted
against 'net'. And yes, Fixes: tags are quite desirable as well.
^ permalink raw reply
* [PATCH net 1/9] net: ena: fix rare uncompleted admin command false alarm
From: netanel @ 2017-06-11 12:42 UTC (permalink / raw)
To: davem, netdev
Cc: Netanel Belgazal, dwmw, zorik, matua, saeedb, msw, aliguori,
nafea, evgenys
In-Reply-To: <1497184971-28178-1-git-send-email-netanel@amazon.com>
From: Netanel Belgazal <netanel@amazon.com>
The current flow to detect admin completion is:
while (command_not_completed) {
if (timeout)
error
check_for_completion()
sleep()
}
So in case the sleep took more than the timeout
(in case the thread/workqueue was not scheduled due to higher priority
task or prolonged VMexit), the driver can detect a stall even if
the completion is present.
The fix changes the order of this function to first check for
completion and only after that check if the timeout expired.
Fixes: 1738cd3ed342 ("Add a driver for Amazon Elastic Network Adapters (ENA)")
Signed-off-by: Netanel Belgazal <netanel@amazon.com>
---
drivers/net/ethernet/amazon/ena/ena_com.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/amazon/ena/ena_com.c b/drivers/net/ethernet/amazon/ena/ena_com.c
index 08d11cede9c9..e1c2fab6292f 100644
--- a/drivers/net/ethernet/amazon/ena/ena_com.c
+++ b/drivers/net/ethernet/amazon/ena/ena_com.c
@@ -508,15 +508,20 @@ static int ena_com_comp_status_to_errno(u8 comp_status)
static int ena_com_wait_and_process_admin_cq_polling(struct ena_comp_ctx *comp_ctx,
struct ena_com_admin_queue *admin_queue)
{
- unsigned long flags;
- u32 start_time;
+ unsigned long flags, timeout;
int ret;
- start_time = ((u32)jiffies_to_usecs(jiffies));
+ timeout = jiffies + ADMIN_CMD_TIMEOUT_US;
+
+ while (1) {
+ spin_lock_irqsave(&admin_queue->q_lock, flags);
+ ena_com_handle_admin_completion(admin_queue);
+ spin_unlock_irqrestore(&admin_queue->q_lock, flags);
- while (comp_ctx->status == ENA_CMD_SUBMITTED) {
- if ((((u32)jiffies_to_usecs(jiffies)) - start_time) >
- ADMIN_CMD_TIMEOUT_US) {
+ if (comp_ctx->status != ENA_CMD_SUBMITTED)
+ break;
+
+ if (time_is_before_jiffies(timeout)) {
pr_err("Wait for completion (polling) timeout\n");
/* ENA didn't have any completion */
spin_lock_irqsave(&admin_queue->q_lock, flags);
@@ -528,10 +533,6 @@ static int ena_com_wait_and_process_admin_cq_polling(struct ena_comp_ctx *comp_c
goto err;
}
- spin_lock_irqsave(&admin_queue->q_lock, flags);
- ena_com_handle_admin_completion(admin_queue);
- spin_unlock_irqrestore(&admin_queue->q_lock, flags);
-
msleep(100);
}
--
2.7.4
^ permalink raw reply related
* [PATCH net 0/9] Bugs fixes in ena ethernet driver
From: netanel @ 2017-06-11 12:42 UTC (permalink / raw)
To: davem, netdev
Cc: Netanel Belgazal, dwmw, zorik, matua, saeedb, msw, aliguori,
nafea, evgenys
From: Netanel Belgazal <netanel@amazon.com>
This patchset contains fixes for the bugs that were discovered so far.
Netanel Belgazal (9):
net: ena: fix rare uncompleted admin command false alarm
net: ena: fix bug that might cause hang after consecutive open/close
interface.
net: ena: add missing return when ena_com_get_io_handlers() fails
net: ena: fix race condition between submit and completion admin
command
net: ena: add missing unmap bars on device removal
net: ena: fix theoretical Rx hang on low memory systems
net: ena: disable admin msix while working in polling mode
net: ena: bug fix in lost tx packets detection mechanism
net: ena: update ena driver to version 1.1.7
drivers/net/ethernet/amazon/ena/ena_com.c | 35 +++--
drivers/net/ethernet/amazon/ena/ena_ethtool.c | 2 +-
drivers/net/ethernet/amazon/ena/ena_netdev.c | 179 +++++++++++++++++++-------
drivers/net/ethernet/amazon/ena/ena_netdev.h | 18 ++-
4 files changed, 169 insertions(+), 65 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH net 3/9] net: ena: add missing return when ena_com_get_io_handlers() fails
From: netanel @ 2017-06-11 12:42 UTC (permalink / raw)
To: davem, netdev
Cc: Netanel Belgazal, dwmw, zorik, matua, saeedb, msw, aliguori,
nafea, evgenys
In-Reply-To: <1497184971-28178-1-git-send-email-netanel@amazon.com>
From: Netanel Belgazal <netanel@amazon.com>
Fixes: 1738cd3ed342 ("Add a driver for Amazon Elastic Network Adapters (ENA)")
Signed-off-by: Netanel Belgazal <netanel@amazon.com>
---
drivers/net/ethernet/amazon/ena/ena_netdev.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
index 0e3c60c7eccf..1e71e89e1e18 100644
--- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
+++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
@@ -1543,6 +1543,7 @@ static int ena_create_io_tx_queue(struct ena_adapter *adapter, int qid)
"Failed to get TX queue handlers. TX queue num %d rc: %d\n",
qid, rc);
ena_com_destroy_io_queue(ena_dev, ena_qid);
+ return rc;
}
ena_com_update_numa_node(tx_ring->ena_com_io_cq, ctx.numa_node);
@@ -1607,6 +1608,7 @@ static int ena_create_io_rx_queue(struct ena_adapter *adapter, int qid)
"Failed to get RX queue handlers. RX queue num %d rc: %d\n",
qid, rc);
ena_com_destroy_io_queue(ena_dev, ena_qid);
+ return rc;
}
ena_com_update_numa_node(rx_ring->ena_com_io_cq, ctx.numa_node);
--
2.7.4
^ permalink raw reply related
* [PATCH net 2/9] net: ena: fix bug that might cause hang after consecutive open/close interface.
From: netanel @ 2017-06-11 12:42 UTC (permalink / raw)
To: davem, netdev
Cc: Netanel Belgazal, dwmw, zorik, matua, saeedb, msw, aliguori,
nafea, evgenys
In-Reply-To: <1497184971-28178-1-git-send-email-netanel@amazon.com>
From: Netanel Belgazal <netanel@amazon.com>
Fixing a bug that the driver does not unmask the IO interrupts
in ndo_open():
occasionally, the MSI-X interrupt (for one or more IO queues)
can be masked when ndo_close() was called.
If that is followed by ndo open(),
then the MSI-X will be still masked so no interrupt
will be received by the driver.
Fixes: 1738cd3ed342 ("Add a driver for Amazon Elastic Network Adapters (ENA)")
Signed-off-by: Netanel Belgazal <netanel@amazon.com>
---
drivers/net/ethernet/amazon/ena/ena_netdev.c | 41 ++++++++++++++++++----------
1 file changed, 26 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
index 7c1214d78855..0e3c60c7eccf 100644
--- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
+++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
@@ -1078,6 +1078,26 @@ inline void ena_adjust_intr_moderation(struct ena_ring *rx_ring,
rx_ring->per_napi_bytes = 0;
}
+static inline void ena_unmask_interrupt(struct ena_ring *tx_ring,
+ struct ena_ring *rx_ring)
+{
+ struct ena_eth_io_intr_reg intr_reg;
+
+ /* Update intr register: rx intr delay,
+ * tx intr delay and interrupt unmask
+ */
+ ena_com_update_intr_reg(&intr_reg,
+ rx_ring->smoothed_interval,
+ tx_ring->smoothed_interval,
+ true);
+
+ /* It is a shared MSI-X.
+ * Tx and Rx CQ have pointer to it.
+ * So we use one of them to reach the intr reg
+ */
+ ena_com_unmask_intr(rx_ring->ena_com_io_cq, &intr_reg);
+}
+
static inline void ena_update_ring_numa_node(struct ena_ring *tx_ring,
struct ena_ring *rx_ring)
{
@@ -1108,7 +1128,6 @@ static int ena_io_poll(struct napi_struct *napi, int budget)
{
struct ena_napi *ena_napi = container_of(napi, struct ena_napi, napi);
struct ena_ring *tx_ring, *rx_ring;
- struct ena_eth_io_intr_reg intr_reg;
u32 tx_work_done;
u32 rx_work_done;
@@ -1149,22 +1168,9 @@ static int ena_io_poll(struct napi_struct *napi, int budget)
if (ena_com_get_adaptive_moderation_enabled(rx_ring->ena_dev))
ena_adjust_intr_moderation(rx_ring, tx_ring);
- /* Update intr register: rx intr delay,
- * tx intr delay and interrupt unmask
- */
- ena_com_update_intr_reg(&intr_reg,
- rx_ring->smoothed_interval,
- tx_ring->smoothed_interval,
- true);
-
- /* It is a shared MSI-X.
- * Tx and Rx CQ have pointer to it.
- * So we use one of them to reach the intr reg
- */
- ena_com_unmask_intr(rx_ring->ena_com_io_cq, &intr_reg);
+ ena_unmask_interrupt(tx_ring, rx_ring);
}
-
ena_update_ring_numa_node(tx_ring, rx_ring);
ret = rx_work_done;
@@ -1485,6 +1491,11 @@ static int ena_up_complete(struct ena_adapter *adapter)
ena_napi_enable_all(adapter);
+ /* Enable completion queues interrupt */
+ for (i = 0; i < adapter->num_queues; i++)
+ ena_unmask_interrupt(&adapter->tx_ring[i],
+ &adapter->rx_ring[i]);
+
/* schedule napi in case we had pending packets
* from the last time we disable napi
*/
--
2.7.4
^ permalink raw reply related
* [PATCH net 4/9] net: ena: fix race condition between submit and completion admin command
From: netanel @ 2017-06-11 12:42 UTC (permalink / raw)
To: davem, netdev
Cc: Netanel Belgazal, dwmw, zorik, matua, saeedb, msw, aliguori,
nafea, evgenys
In-Reply-To: <1497184971-28178-1-git-send-email-netanel@amazon.com>
From: Netanel Belgazal <netanel@amazon.com>
Bug:
"Completion context is occupied" error printout will be noticed in
dmesg.
This error will cause the admin command to fail, which will lead to
an ena_probe() failure or a watchdog reset (depends on which admin
command failed).
Root cause:
__ena_com_submit_admin_cmd() is the function that submits new entries to
the admin queue.
The function have a check that makes sure the queue is not full and the
function does not override any outstanding command.
It uses head and tail indexes for this check.
The head is increased by ena_com_handle_admin_completion() which runs
from interrupt context, and the tail index is increased by the submit
function (the function is running under ->q_lock, so there is no risk
of multithread increment).
Each command is associated with a completion context. This context
allocated before call to __ena_com_submit_admin_cmd() and freed by
ena_com_wait_and_process_admin_cq_interrupts(), right after the command
was completed.
This can lead to a state where the head was increased, the check passed,
but the completion context is still in use.
Solution:
Use the atomic variable ->outstanding_cmds instead of using the head and
the tail indexes.
This variable is safe for use since it is bumped in get_comp_ctx() in
__ena_com_submit_admin_cmd() and is freed by comp_ctxt_release()
Fixes: 1738cd3ed342 ("Add a driver for Amazon Elastic Network Adapters (ENA)")
Signed-off-by: Netanel Belgazal <netanel@amazon.com>
---
drivers/net/ethernet/amazon/ena/ena_com.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/amazon/ena/ena_com.c b/drivers/net/ethernet/amazon/ena/ena_com.c
index e1c2fab6292f..ea60b9e67acb 100644
--- a/drivers/net/ethernet/amazon/ena/ena_com.c
+++ b/drivers/net/ethernet/amazon/ena/ena_com.c
@@ -232,11 +232,9 @@ static struct ena_comp_ctx *__ena_com_submit_admin_cmd(struct ena_com_admin_queu
tail_masked = admin_queue->sq.tail & queue_size_mask;
/* In case of queue FULL */
- cnt = admin_queue->sq.tail - admin_queue->sq.head;
+ cnt = atomic_read(&admin_queue->outstanding_cmds);
if (cnt >= admin_queue->q_depth) {
- pr_debug("admin queue is FULL (tail %d head %d depth: %d)\n",
- admin_queue->sq.tail, admin_queue->sq.head,
- admin_queue->q_depth);
+ pr_debug("admin queue is full.\n");
admin_queue->stats.out_of_space++;
return ERR_PTR(-ENOSPC);
}
--
2.7.4
^ permalink raw reply related
* [PATCH net 8/9] net: ena: bug fix in lost tx packets detection mechanism
From: netanel @ 2017-06-11 12:42 UTC (permalink / raw)
To: davem, netdev
Cc: Netanel Belgazal, dwmw, zorik, matua, saeedb, msw, aliguori,
nafea, evgenys
In-Reply-To: <1497184971-28178-1-git-send-email-netanel@amazon.com>
From: Netanel Belgazal <netanel@amazon.com>
check_for_missing_tx_completions() is called from a timer
task and looking for lost tx packets.
The old implementation accumulate all the lost tx packets
and did not check if those packets were retrieved on a later stage.
This cause to a situation where the driver reset
the device for no reason.
Fixes: 1738cd3ed342 ("Add a driver for Amazon Elastic Network Adapters (ENA)")
Signed-off-by: Netanel Belgazal <netanel@amazon.com>
---
drivers/net/ethernet/amazon/ena/ena_ethtool.c | 1 -
drivers/net/ethernet/amazon/ena/ena_netdev.c | 66 +++++++++++++++------------
drivers/net/ethernet/amazon/ena/ena_netdev.h | 14 +++++-
3 files changed, 50 insertions(+), 31 deletions(-)
diff --git a/drivers/net/ethernet/amazon/ena/ena_ethtool.c b/drivers/net/ethernet/amazon/ena/ena_ethtool.c
index 533b2fbdeef1..3ee55e2fd694 100644
--- a/drivers/net/ethernet/amazon/ena/ena_ethtool.c
+++ b/drivers/net/ethernet/amazon/ena/ena_ethtool.c
@@ -80,7 +80,6 @@ static const struct ena_stats ena_stats_tx_strings[] = {
ENA_STAT_TX_ENTRY(tx_poll),
ENA_STAT_TX_ENTRY(doorbells),
ENA_STAT_TX_ENTRY(prepare_ctx_err),
- ENA_STAT_TX_ENTRY(missing_tx_comp),
ENA_STAT_TX_ENTRY(bad_req_id),
};
diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
index 3c366bfbbab1..4f16ed38bcf3 100644
--- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
+++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
@@ -1995,6 +1995,7 @@ static netdev_tx_t ena_start_xmit(struct sk_buff *skb, struct net_device *dev)
tx_info->tx_descs = nb_hw_desc;
tx_info->last_jiffies = jiffies;
+ tx_info->print_once = 0;
tx_ring->next_to_use = ENA_TX_RING_IDX_NEXT(next_to_use,
tx_ring->ring_size);
@@ -2564,13 +2565,44 @@ static void ena_fw_reset_device(struct work_struct *work)
"Reset attempt failed. Can not reset the device\n");
}
-static void check_for_missing_tx_completions(struct ena_adapter *adapter)
+static int check_missing_comp_in_queue(struct ena_adapter *adapter,
+ struct ena_ring *tx_ring)
{
struct ena_tx_buffer *tx_buf;
unsigned long last_jiffies;
+ u32 missed_tx = 0;
+ int i;
+
+ for (i = 0; i < tx_ring->ring_size; i++) {
+ tx_buf = &tx_ring->tx_buffer_info[i];
+ last_jiffies = tx_buf->last_jiffies;
+ if (unlikely(last_jiffies &&
+ time_is_before_jiffies(last_jiffies + TX_TIMEOUT))) {
+ if (!tx_buf->print_once)
+ netif_notice(adapter, tx_err, adapter->netdev,
+ "Found a Tx that wasn't completed on time, qid %d, index %d.\n",
+ tx_ring->qid, i);
+
+ tx_buf->print_once = 1;
+ missed_tx++;
+
+ if (unlikely(missed_tx > MAX_NUM_OF_TIMEOUTED_PACKETS)) {
+ netif_err(adapter, tx_err, adapter->netdev,
+ "The number of lost tx completions is above the threshold (%d > %d). Reset the device\n",
+ missed_tx, MAX_NUM_OF_TIMEOUTED_PACKETS);
+ set_bit(ENA_FLAG_TRIGGER_RESET, &adapter->flags);
+ return -EIO;
+ }
+ }
+ }
+
+ return 0;
+}
+
+static void check_for_missing_tx_completions(struct ena_adapter *adapter)
+{
struct ena_ring *tx_ring;
- int i, j, budget;
- u32 missed_tx;
+ int i, budget, rc;
/* Make sure the driver doesn't turn the device in other process */
smp_rmb();
@@ -2586,31 +2618,9 @@ static void check_for_missing_tx_completions(struct ena_adapter *adapter)
for (i = adapter->last_monitored_tx_qid; i < adapter->num_queues; i++) {
tx_ring = &adapter->tx_ring[i];
- for (j = 0; j < tx_ring->ring_size; j++) {
- tx_buf = &tx_ring->tx_buffer_info[j];
- last_jiffies = tx_buf->last_jiffies;
- if (unlikely(last_jiffies && time_is_before_jiffies(last_jiffies + TX_TIMEOUT))) {
- netif_notice(adapter, tx_err, adapter->netdev,
- "Found a Tx that wasn't completed on time, qid %d, index %d.\n",
- tx_ring->qid, j);
-
- u64_stats_update_begin(&tx_ring->syncp);
- missed_tx = tx_ring->tx_stats.missing_tx_comp++;
- u64_stats_update_end(&tx_ring->syncp);
-
- /* Clear last jiffies so the lost buffer won't
- * be counted twice.
- */
- tx_buf->last_jiffies = 0;
-
- if (unlikely(missed_tx > MAX_NUM_OF_TIMEOUTED_PACKETS)) {
- netif_err(adapter, tx_err, adapter->netdev,
- "The number of lost tx completion is above the threshold (%d > %d). Reset the device\n",
- missed_tx, MAX_NUM_OF_TIMEOUTED_PACKETS);
- set_bit(ENA_FLAG_TRIGGER_RESET, &adapter->flags);
- }
- }
- }
+ rc = check_missing_comp_in_queue(adapter, tx_ring);
+ if (unlikely(rc))
+ return;
budget--;
if (!budget)
diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.h b/drivers/net/ethernet/amazon/ena/ena_netdev.h
index 8828f1d6dd22..88b5e5612338 100644
--- a/drivers/net/ethernet/amazon/ena/ena_netdev.h
+++ b/drivers/net/ethernet/amazon/ena/ena_netdev.h
@@ -146,7 +146,18 @@ struct ena_tx_buffer {
u32 tx_descs;
/* num of buffers used by this skb */
u32 num_of_bufs;
- /* Save the last jiffies to detect missing tx packets */
+
+ /* Used for detect missing tx packets to limit the number of prints */
+ u32 print_once;
+ /* Save the last jiffies to detect missing tx packets
+ *
+ * sets to non zero value on ena_start_xmit and set to zero on
+ * napi and timer_Service_routine.
+ *
+ * while this value is not protected by lock,
+ * a given packet is not expected to be handled by ena_start_xmit
+ * and by napi/timer_service at the same time.
+ */
unsigned long last_jiffies;
struct ena_com_buf bufs[ENA_PKT_MAX_BUFS];
} ____cacheline_aligned;
@@ -170,7 +181,6 @@ struct ena_stats_tx {
u64 napi_comp;
u64 tx_poll;
u64 doorbells;
- u64 missing_tx_comp;
u64 bad_req_id;
};
--
2.7.4
^ permalink raw reply related
* [PATCH net 6/9] net: ena: fix theoretical Rx hang on low memory systems
From: netanel @ 2017-06-11 12:42 UTC (permalink / raw)
To: davem, netdev
Cc: Netanel Belgazal, dwmw, zorik, matua, saeedb, msw, aliguori,
nafea, evgenys
In-Reply-To: <1497184971-28178-1-git-send-email-netanel@amazon.com>
From: Netanel Belgazal <netanel@amazon.com>
For the rare case where the device runs out of free rx buffer
descriptors (in case of pressure on kernel memory),
and the napi handler continuously fail to refill new Rx descriptors
until device rx queue totally runs out of all free rx buffers
to post incoming packet, leading to a deadlock:
* The device won't send interrupts since all the new
Rx packets will be dropped.
* The napi handler won't try to allocate new Rx descriptors
since allocation is part of NAPI that's not being invoked any more
The fix involves detecting this scenario and rescheduling NAPI
(to refill buffers) by the keepalive/watchdog task.
Fixes: 1738cd3ed342 ("Add a driver for Amazon Elastic Network Adapters (ENA)")
Signed-off-by: Netanel Belgazal <netanel@amazon.com>
---
drivers/net/ethernet/amazon/ena/ena_ethtool.c | 1 +
drivers/net/ethernet/amazon/ena/ena_netdev.c | 55 +++++++++++++++++++++++++++
drivers/net/ethernet/amazon/ena/ena_netdev.h | 2 +
3 files changed, 58 insertions(+)
diff --git a/drivers/net/ethernet/amazon/ena/ena_ethtool.c b/drivers/net/ethernet/amazon/ena/ena_ethtool.c
index 67b2338f8fb3..533b2fbdeef1 100644
--- a/drivers/net/ethernet/amazon/ena/ena_ethtool.c
+++ b/drivers/net/ethernet/amazon/ena/ena_ethtool.c
@@ -94,6 +94,7 @@ static const struct ena_stats ena_stats_rx_strings[] = {
ENA_STAT_RX_ENTRY(dma_mapping_err),
ENA_STAT_RX_ENTRY(bad_desc_num),
ENA_STAT_RX_ENTRY(rx_copybreak_pkt),
+ ENA_STAT_RX_ENTRY(empty_rx_ring),
};
static const struct ena_stats ena_stats_ena_com_strings[] = {
diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
index 4e9fbddd3b47..3c366bfbbab1 100644
--- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
+++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
@@ -190,6 +190,7 @@ static void ena_init_io_rings(struct ena_adapter *adapter)
rxr->sgl_size = adapter->max_rx_sgl_size;
rxr->smoothed_interval =
ena_com_get_nonadaptive_moderation_interval_rx(ena_dev);
+ rxr->empty_rx_queue = 0;
}
}
@@ -2619,6 +2620,58 @@ static void check_for_missing_tx_completions(struct ena_adapter *adapter)
adapter->last_monitored_tx_qid = i % adapter->num_queues;
}
+/* trigger napi schedule after 2 consecutive detections */
+#define EMPTY_RX_REFILL 2
+/* For the rare case where the device runs out of Rx descriptors and the
+ * napi handler failed to refill new Rx descriptors (due to a lack of memory
+ * for example).
+ * This case will lead to a deadlock:
+ * The device won't send interrupts since all the new Rx packets will be dropped
+ * The napi handler won't allocate new Rx descriptors so the device will be
+ * able to send new packets.
+ *
+ * This scenario can happen when the kernel's vm.min_free_kbytes is too small.
+ * It is recommended to have at least 512MB, with a minimum of 128MB for
+ * constrained environment).
+ *
+ * When such a situation is detected - Reschedule napi
+ */
+static void check_for_empty_rx_ring(struct ena_adapter *adapter)
+{
+ struct ena_ring *rx_ring;
+ int i, refill_required;
+
+ if (!test_bit(ENA_FLAG_DEV_UP, &adapter->flags))
+ return;
+
+ if (test_bit(ENA_FLAG_TRIGGER_RESET, &adapter->flags))
+ return;
+
+ for (i = 0; i < adapter->num_queues; i++) {
+ rx_ring = &adapter->rx_ring[i];
+
+ refill_required =
+ ena_com_sq_empty_space(rx_ring->ena_com_io_sq);
+ if (unlikely(refill_required == (rx_ring->ring_size - 1))) {
+ rx_ring->empty_rx_queue++;
+
+ if (rx_ring->empty_rx_queue >= EMPTY_RX_REFILL) {
+ u64_stats_update_begin(&rx_ring->syncp);
+ rx_ring->rx_stats.empty_rx_ring++;
+ u64_stats_update_end(&rx_ring->syncp);
+
+ netif_err(adapter, drv, adapter->netdev,
+ "trigger refill for ring %d\n", i);
+
+ napi_schedule(rx_ring->napi);
+ rx_ring->empty_rx_queue = 0;
+ }
+ } else {
+ rx_ring->empty_rx_queue = 0;
+ }
+ }
+}
+
/* Check for keep alive expiration */
static void check_for_missing_keep_alive(struct ena_adapter *adapter)
{
@@ -2673,6 +2726,8 @@ static void ena_timer_service(unsigned long data)
check_for_missing_tx_completions(adapter);
+ check_for_empty_rx_ring(adapter);
+
if (debug_area)
ena_dump_stats_to_buf(adapter, debug_area);
diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.h b/drivers/net/ethernet/amazon/ena/ena_netdev.h
index 0e22bce6239d..8828f1d6dd22 100644
--- a/drivers/net/ethernet/amazon/ena/ena_netdev.h
+++ b/drivers/net/ethernet/amazon/ena/ena_netdev.h
@@ -184,6 +184,7 @@ struct ena_stats_rx {
u64 dma_mapping_err;
u64 bad_desc_num;
u64 rx_copybreak_pkt;
+ u64 empty_rx_ring;
};
struct ena_ring {
@@ -231,6 +232,7 @@ struct ena_ring {
struct ena_stats_tx tx_stats;
struct ena_stats_rx rx_stats;
};
+ int empty_rx_queue;
} ____cacheline_aligned;
struct ena_stats_dev {
--
2.7.4
^ permalink raw reply related
* [PATCH net 7/9] net: ena: disable admin msix while working in polling mode
From: netanel @ 2017-06-11 12:42 UTC (permalink / raw)
To: davem, netdev
Cc: Netanel Belgazal, dwmw, zorik, matua, saeedb, msw, aliguori,
nafea, evgenys
In-Reply-To: <1497184971-28178-1-git-send-email-netanel@amazon.com>
From: Netanel Belgazal <netanel@amazon.com>
Fixes: 1738cd3ed342 ("Add a driver for Amazon Elastic Network Adapters (ENA)")
Signed-off-by: Netanel Belgazal <netanel@amazon.com>
---
drivers/net/ethernet/amazon/ena/ena_com.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/net/ethernet/amazon/ena/ena_com.c b/drivers/net/ethernet/amazon/ena/ena_com.c
index ea60b9e67acb..f5b237e0bd60 100644
--- a/drivers/net/ethernet/amazon/ena/ena_com.c
+++ b/drivers/net/ethernet/amazon/ena/ena_com.c
@@ -61,6 +61,8 @@
#define ENA_MMIO_READ_TIMEOUT 0xFFFFFFFF
+#define ENA_REGS_ADMIN_INTR_MASK 1
+
/*****************************************************************************/
/*****************************************************************************/
/*****************************************************************************/
@@ -1454,6 +1456,12 @@ void ena_com_admin_destroy(struct ena_com_dev *ena_dev)
void ena_com_set_admin_polling_mode(struct ena_com_dev *ena_dev, bool polling)
{
+ u32 mask_value = 0;
+
+ if (polling)
+ mask_value = ENA_REGS_ADMIN_INTR_MASK;
+
+ writel(mask_value, ena_dev->reg_bar + ENA_REGS_INTR_MASK_OFF);
ena_dev->admin_queue.polling = polling;
}
--
2.7.4
^ permalink raw reply related
* [PATCH net 5/9] net: ena: add missing unmap bars on device removal
From: netanel @ 2017-06-11 12:42 UTC (permalink / raw)
To: davem, netdev
Cc: Netanel Belgazal, dwmw, zorik, matua, saeedb, msw, aliguori,
nafea, evgenys
In-Reply-To: <1497184971-28178-1-git-send-email-netanel@amazon.com>
From: Netanel Belgazal <netanel@amazon.com>
This patch also change the mapping functions to devm_ functions
Fixes: 1738cd3ed342 ("Add a driver for Amazon Elastic Network Adapters (ENA)")
Signed-off-by: Netanel Belgazal <netanel@amazon.com>
---
drivers/net/ethernet/amazon/ena/ena_netdev.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
index 1e71e89e1e18..4e9fbddd3b47 100644
--- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
+++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
@@ -2853,6 +2853,11 @@ static void ena_release_bars(struct ena_com_dev *ena_dev, struct pci_dev *pdev)
{
int release_bars;
+ if (ena_dev->mem_bar)
+ devm_iounmap(&pdev->dev, ena_dev->mem_bar);
+
+ devm_iounmap(&pdev->dev, ena_dev->reg_bar);
+
release_bars = pci_select_bars(pdev, IORESOURCE_MEM) & ENA_BAR_MASK;
pci_release_selected_regions(pdev, release_bars);
}
@@ -2940,8 +2945,9 @@ static int ena_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
goto err_free_ena_dev;
}
- ena_dev->reg_bar = ioremap(pci_resource_start(pdev, ENA_REG_BAR),
- pci_resource_len(pdev, ENA_REG_BAR));
+ ena_dev->reg_bar = devm_ioremap(&pdev->dev,
+ pci_resource_start(pdev, ENA_REG_BAR),
+ pci_resource_len(pdev, ENA_REG_BAR));
if (!ena_dev->reg_bar) {
dev_err(&pdev->dev, "failed to remap regs bar\n");
rc = -EFAULT;
@@ -2961,8 +2967,9 @@ static int ena_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
ena_set_push_mode(pdev, ena_dev, &get_feat_ctx);
if (ena_dev->tx_mem_queue_type == ENA_ADMIN_PLACEMENT_POLICY_DEV) {
- ena_dev->mem_bar = ioremap_wc(pci_resource_start(pdev, ENA_MEM_BAR),
- pci_resource_len(pdev, ENA_MEM_BAR));
+ ena_dev->mem_bar = devm_ioremap_wc(&pdev->dev,
+ pci_resource_start(pdev, ENA_MEM_BAR),
+ pci_resource_len(pdev, ENA_MEM_BAR));
if (!ena_dev->mem_bar) {
rc = -EFAULT;
goto err_device_destroy;
--
2.7.4
^ permalink raw reply related
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