* Re: [PATCH v2 net-next 1/1] driver: tun: Forbid to set IFF_TUN and IFF_TAP at the same time
From: Gao Feng @ 2016-10-21 11:32 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S. Miller, jasowang, edumazet, pabeni,
Linux Kernel Network Developers
In-Reply-To: <1477049287.7065.51.camel@edumazet-glaptop3.roam.corp.google.com>
Hi Eric,
On Fri, Oct 21, 2016 at 7:28 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Fri, 2016-10-21 at 19:02 +0800, fgao@ikuai8.com wrote:
>> From: Gao Feng <fgao@ikuai8.com>
>>
>> Current tun driver permits the ifr_flags is set with IFF_TUN and
>> IFF_TAP at the same time. But actually there is only IFF_TUN flag
>> works. And it does not make sense these two flags are set, so add
>> this check.
>>
>> Signed-off-by: Gao Feng <fgao@ikuai8.com>
>> ---
>> v2: Remove useless {}
>> v1: Initial patch
>>
>> drivers/net/tun.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
>> index 8093e39..faaa189 100644
>> --- a/drivers/net/tun.c
>> +++ b/drivers/net/tun.c
>> @@ -1752,6 +1752,9 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
>> if (err < 0)
>> return err;
>>
>> + if ((ifr->ifr_flags & (IFF_TUN | IFF_TAP)) == (IFF_TUN | IFF_TAP))
>> + return -EINVAL;
>> +
>> /* Set dev type */
>> if (ifr->ifr_flags & IFF_TUN) {
>> /* TUN device */
>
>
> This might break some applications.
Yes. I consider about this case.
But I think there should be very least applications which set these
two flags at the same time.
>
> It might be too late to add this check without a grace period.
>
>
>
Yes, It needs some discussions.
Regards
Feng
^ permalink raw reply
* [PATCH net] net: fec: Call swap_buffer() prior to IP header alignment
From: Fabio Estevam @ 2016-10-21 11:34 UTC (permalink / raw)
To: davem; +Cc: fugang.duan, henri.roosen, eric, kernel, rmk, netdev,
Fabio Estevam
From: Fabio Estevam <fabio.estevam@nxp.com>
Commit 3ac72b7b63d5 ("net: fec: align IP header in hardware") breaks
networking on mx28.
There is an erratum on mx28 (ENGR121613 - ENET big endian mode
not compatible with ARM little endian) that requires an additional
byte-swap operation to workaround this problem.
So call swap_buffer() prior to performing the IP header alignment
to restore network functionality on mx28.
Fixes: 3ac72b7b63d5 ("net: fec: align IP header in hardware")
Reported-and-tested-by: Henri Roosen <henri.roosen@ginzinger.com>
Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
---
drivers/net/ethernet/freescale/fec_main.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 4ce8179..3c888be 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -1430,14 +1430,14 @@ fec_enet_rx_queue(struct net_device *ndev, int budget, u16 queue_id)
skb_put(skb, pkt_len - 4);
data = skb->data;
+ if (!is_copybreak && need_swap)
+ swap_buffer(data, pkt_len);
+
#if !defined(CONFIG_M5272)
if (fep->quirks & FEC_QUIRK_HAS_RACC)
data = skb_pull_inline(skb, 2);
#endif
- if (!is_copybreak && need_swap)
- swap_buffer(data, pkt_len);
-
/* Extract the enhanced buffer descriptor */
ebdp = NULL;
if (fep->bufdesc_ex)
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] net: sched: make default fifo qdiscs appear in the dump
From: Eric Dumazet @ 2016-10-21 11:36 UTC (permalink / raw)
To: Jiri Kosina
Cc: Jamal Hadi Salim, Phil Sutter, Cong Wang, Daniel Borkmann,
David S. Miller, linux-kernel, netdev
In-Reply-To: <alpine.LNX.2.00.1610211024400.31629@cbobk.fhfr.pm>
On Fri, 2016-10-21 at 10:45 +0200, Jiri Kosina wrote:
> The original reason [1] for having hidden qdiscs (potential scalability
> issues in qdisc_match_from_root() with single linked list in case of large
> amount of qdiscs) has been invalidated by 59cc1f61f0 ("net: sched: convert
> qdisc linked list to hashtable").
>
> This allows us for bringing more clarity and determinism into the dump by
> making default pfifo qdiscs visible.
>
> [1] http://lkml.kernel.org/r/1460732328.10638.74.camel@edumazet-glaptop3.roam.corp.google.com
>
> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
> ---
Some of us are dealing with huge HTB hierarchies, so adding default fifo
in the dump will add more data pumped from the kernel.
BwE [1] for instance dumps qdisc/classes every 5 seconds.
I guess we'll need to not pull this patch in our kernels.
This reminds me of an HTB patch to avoid dumping rate estimates for HTB.
I will send it today.
[1]
http://static.googleusercontent.com/media/research.google.com/en//pubs/archive/43838.pdf
^ permalink raw reply
* Re: [PATCH v3 net-next 8/8] net: qualcomm: add QCA7000 UART driver
From: Stefan Wahren @ 2016-10-21 11:38 UTC (permalink / raw)
To: David Miller; +Cc: gregkh, jslaby, netdev, linux-kernel
In-Reply-To: <20161018.143025.284251297276702898.davem@davemloft.net>
Am 18.10.2016 um 20:30 schrieb David Miller:
> From: Stefan Wahren <stefan.wahren@i2se.com>
> Date: Tue, 18 Oct 2016 13:27:34 +0200
>
>> +void
>> +qca_tty_receive(struct tty_struct *tty, const unsigned char *cp, char *fp,
>> + int count)
>> +{
>> + struct qcauart *qca = tty->disc_data;
>> + struct net_device_stats *n_stats = &qca->net_dev->stats;
>> + int dropped = 0;
> Please order local variable declarations from longest to shortest line.
Even if the current second init depends on the first one?
^ permalink raw reply
* Re: [PATCH net-next v5 2/3] udp: implement memory accounting helpers
From: Paolo Abeni @ 2016-10-21 11:43 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, David S. Miller, James Morris,
Trond Myklebust, Alexander Duyck, Daniel Borkmann, Eric Dumazet,
Tom Herbert, Hannes Frederic Sowa, Edward Cree,
linux-nfs-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1476997834.7065.35.camel-XN9IlZ5yJG9HTL0Zs8A6p+yfmBU6pStAUsxypvmhUTTZJqsBc5GL+g@public.gmane.org>
On Thu, 2016-10-20 at 14:10 -0700, Eric Dumazet wrote:
> On Thu, 2016-10-20 at 22:31 +0200, Paolo Abeni wrote:
>
> > +
> > +int __udp_enqueue_schedule_skb(struct sock *sk, struct sk_buff *skb)
> > +{
> > + struct sk_buff_head *list = &sk->sk_receive_queue;
> > + int rmem, delta, amt, err = -ENOMEM;
> > + int size = skb->truesize;
> > +
> > + /* try to avoid the costly atomic add/sub pair when the receive
> > + * queue is full; always allow at least a packet
> > + */
> > + rmem = atomic_read(&sk->sk_rmem_alloc);
> > + if (rmem && (rmem + size > sk->sk_rcvbuf))
> > + goto drop;
> > +
> > + /* we drop only if the receive buf is full and the receive
> > + * queue contains some other skb
> > + */
> > + rmem = atomic_add_return(size, &sk->sk_rmem_alloc);
> > + if ((rmem > sk->sk_rcvbuf) && (rmem > size))
> > + goto uncharge_drop;
> > +
> > + skb_orphan(skb);
>
> Minor point :
>
> UDP should already have orphaned skbs ? (it uses skb_steal_sock())
You are right, the skb_orphan() call is redundant.
I'll send the v6 soon.
Thank you,
Paolo
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] net: stmmac: Add OXNAS Glue Driver
From: Giuseppe CAVALLARO @ 2016-10-21 11:53 UTC (permalink / raw)
To: Joachim Eastwood, Neil Armstrong
Cc: alexandre.torgue, netdev, linux-oxnas,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, devicetree
In-Reply-To: <CAGhQ9Vxe8fe8kfF_WY40xdh6YX9TuxQ3-Dd6pBG=AHpLtrHnMA@mail.gmail.com>
Hello
some my minor cents below
On 10/21/2016 12:20 PM, Joachim Eastwood wrote:
> Hi Neil,
>
> On 21 October 2016 at 10:44, Neil Armstrong <narmstrong@baylibre.com> wrote:
>> Add Synopsys Designware MAC Glue layer for the Oxford Semiconductor OX820.
>>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>> .../devicetree/bindings/net/oxnas-dwmac.txt | 44 +++++
>> drivers/net/ethernet/stmicro/stmmac/Kconfig | 11 ++
>> drivers/net/ethernet/stmicro/stmmac/Makefile | 1 +
>> drivers/net/ethernet/stmicro/stmmac/dwmac-oxnas.c | 219 +++++++++++++++++++++
>> 4 files changed, 275 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/net/oxnas-dwmac.txt
>> create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-oxnas.c
>>
>> Changes since RFC at https://patchwork.kernel.org/patch/9387257 :
>> - Drop init/exit callbacks
>> - Implement proper remove and PM callback
>> - Call init from probe
>> - Disable/Unprepare clock if stmmac probe fails
>
> <snip>
>
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-oxnas.c
>> @@ -0,0 +1,219 @@
>> +/*
>> + * Oxford Semiconductor OXNAS DWMAC glue layer
>> + *
>> + * Copyright (C) 2016 Neil Armstrong <narmstrong@baylibre.com>
>> + * Copyright (C) 2014 Daniel Golle <daniel@makrotopia.org>
>> + * Copyright (C) 2013 Ma Haijun <mahaijuns@gmail.com>
>> + * Copyright (C) 2012 John Crispin <blogic@openwrt.org>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#include <linux/device.h>
>> +#include <linux/io.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/stmmac.h>
>> +
>> +#include "stmmac_platform.h"
>> +
>> +/* System Control regmap offsets */
>> +#define OXNAS_DWMAC_CTRL_REGOFFSET 0x78
>> +#define OXNAS_DWMAC_DELAY_REGOFFSET 0x100
>> +
>> +/* Control Register */
>> +#define DWMAC_CKEN_RX_IN 14
>> +#define DWMAC_CKEN_RXN_OUT 13
>> +#define DWMAC_CKEN_RX_OUT 12
>> +#define DWMAC_CKEN_TX_IN 10
>> +#define DWMAC_CKEN_TXN_OUT 9
>> +#define DWMAC_CKEN_TX_OUT 8
>> +#define DWMAC_RX_SOURCE 7
>> +#define DWMAC_TX_SOURCE 6
>> +#define DWMAC_LOW_TX_SOURCE 4
>> +#define DWMAC_AUTO_TX_SOURCE 3
>> +#define DWMAC_RGMII 2
>> +#define DWMAC_SIMPLE_MUX 1
>> +#define DWMAC_CKEN_GTX 0
>> +
>> +/* Delay register */
>> +#define DWMAC_TX_VARDELAY_SHIFT 0
>> +#define DWMAC_TXN_VARDELAY_SHIFT 8
>> +#define DWMAC_RX_VARDELAY_SHIFT 16
>> +#define DWMAC_RXN_VARDELAY_SHIFT 24
>> +#define DWMAC_TX_VARDELAY(d) ((d) << DWMAC_TX_VARDELAY_SHIFT)
>> +#define DWMAC_TXN_VARDELAY(d) ((d) << DWMAC_TXN_VARDELAY_SHIFT)
>> +#define DWMAC_RX_VARDELAY(d) ((d) << DWMAC_RX_VARDELAY_SHIFT)
>> +#define DWMAC_RXN_VARDELAY(d) ((d) << DWMAC_RXN_VARDELAY_SHIFT)
>> +
>> +struct oxnas_dwmac {
>> + struct device *dev;
>> + struct clk *clk;
>> + struct regmap *regmap;
>> +};
>> +
>> +static int oxnas_dwmac_init(struct oxnas_dwmac *dwmac)
>> +{
>> + unsigned int value;
>> + int ret;
>> +
>> + /* Reset HW here before changing the glue configuration */
>> + ret = device_reset(dwmac->dev);
>> + if (ret)
>> + return ret;
>> +
>> + clk_prepare_enable(dwmac->clk);
>
> You might want to check the return value from clk_prepare_enable() as well.
>
>> +
>> + ret = regmap_read(dwmac->regmap, OXNAS_DWMAC_CTRL_REGOFFSET, &value);
>> + if (ret < 0)
>> + return ret;
>> +
>> + /* Enable GMII_GTXCLK to follow GMII_REFCLK, required for gigabit PHY */
>> + value |= BIT(DWMAC_CKEN_GTX);
>> + /* Use simple mux for 25/125 Mhz clock switching */
>> + value |= BIT(DWMAC_SIMPLE_MUX);
>> + /* set auto switch tx clock source */
>> + value |= BIT(DWMAC_AUTO_TX_SOURCE);
>> + /* enable tx & rx vardelay */
>> + value |= BIT(DWMAC_CKEN_TX_OUT);
>> + value |= BIT(DWMAC_CKEN_TXN_OUT);
>> + value |= BIT(DWMAC_CKEN_TX_IN);
>> + value |= BIT(DWMAC_CKEN_RX_OUT);
>> + value |= BIT(DWMAC_CKEN_RXN_OUT);
>> + value |= BIT(DWMAC_CKEN_RX_IN);
>> + regmap_write(dwmac->regmap, OXNAS_DWMAC_CTRL_REGOFFSET, value);
>> +
>> + /* set tx & rx vardelay */
>> + value = DWMAC_TX_VARDELAY(4);
>> + value |= DWMAC_TXN_VARDELAY(2);
>> + value |= DWMAC_RX_VARDELAY(10);
>> + value |= DWMAC_RXN_VARDELAY(8);
>> + regmap_write(dwmac->regmap, OXNAS_DWMAC_DELAY_REGOFFSET, value);
there is no if condition so, I can suggest you, to hardwire
value with macros instead of computing at runtime:
e.g.
var = DWMAC_VARDELAY where
#define DWMAC_VARDELAY (DWMAC_TX_VARDELAY(4) | ...)
... same for OXNAS_DWMAC_CTRL_REGOFFSET where
BIT(DWMAC_CKEN_ ... ) should be re-organized as macros,
I mean:
#define DWMAC_CKEN_.. BIT(xxx)
>> +
>> + return 0;
>> +}
>> +
>> +static int oxnas_dwmac_probe(struct platform_device *pdev)
>> +{
>> + struct plat_stmmacenet_data *plat_dat;
>> + struct stmmac_resources stmmac_res;
>> + struct device_node *sysctrl;
>> + struct oxnas_dwmac *dwmac;
>> + int ret;
>> +
>> + sysctrl = of_parse_phandle(pdev->dev.of_node, "oxsemi,sys-ctrl", 0);
>> + if (!sysctrl) {
>> + dev_err(&pdev->dev, "failed to get sys-ctrl node\n");
>> + return -EINVAL;
>> + }
>> +
>> + ret = stmmac_get_platform_resources(pdev, &stmmac_res);
>> + if (ret)
>> + return ret;
>> +
>> + plat_dat = stmmac_probe_config_dt(pdev, &stmmac_res.mac);
>> + if (IS_ERR(plat_dat))
>> + return PTR_ERR(plat_dat);
>> +
>> + dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL);
>> + if (!dwmac)
>> + return -ENOMEM;
>> +
>> + dwmac->dev = &pdev->dev;
>> + plat_dat->bsp_priv = dwmac;
>> +
>> + dwmac->regmap = syscon_node_to_regmap(sysctrl);
>> + if (IS_ERR(dwmac->regmap)) {
>> + dev_err(&pdev->dev, "failed to have sysctrl regmap\n");
>> + return PTR_ERR(dwmac->regmap);
>> + }
>> +
>> + dwmac->clk = devm_clk_get(&pdev->dev, "gmac");
>> + if (IS_ERR(dwmac->clk))
>> + return PTR_ERR(dwmac->clk);
>> +
>> + ret = oxnas_dwmac_init(dwmac);
>> + if (ret)
>> + return ret;
>> +
>> + ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
>> + if (ret)
>> + clk_disable_unprepare(dwmac->clk);
>> +
>> + return ret;
>> +}
>> +
>> +static int oxnas_dwmac_remove(struct platform_device *pdev)
>> +{
>> + struct net_device *ndev = platform_get_drvdata(pdev);
>> + struct stmmac_priv *priv = netdev_priv(ndev);
>> + struct oxnas_dwmac *dwmac = priv->plat->bsp_priv;
>
> Instead of this long dance of variables use the get_stmmac_bsp_priv()-helper.
>
> You can take a look at dwmac-meson8b.c for reference.
>
>
>> + int ret = stmmac_dvr_remove(&pdev->dev);
>> +
>> + clk_disable_unprepare(dwmac->clk);
>> +
>> + return ret;
>> +}
>> +
>> +#ifdef CONFIG_PM_SLEEP
>> +static int oxnas_dwmac_suspend(struct device *dev)
>> +{
>> + struct net_device *ndev = dev_get_drvdata(dev);
>> + struct stmmac_priv *priv = netdev_priv(ndev);
>> + struct oxnas_dwmac *dwmac = priv->plat->bsp_priv;
>
> get_stmmac_bsp_priv()
>
>
>> + int ret;
>> +
>> + ret = stmmac_suspend(dev);
>> + clk_disable_unprepare(dwmac->clk);
>> +
>> + return ret;
>> +}
>> +
>> +static int oxnas_dwmac_resume(struct device *dev)
>> +{
>> + struct net_device *ndev = dev_get_drvdata(dev);
>> + struct stmmac_priv *priv = netdev_priv(ndev);
>> + struct oxnas_dwmac *dwmac = priv->plat->bsp_priv;
>
> get_stmmac_bsp_priv()
>
>
>> + int ret;
>> +
>> + ret = oxnas_dwmac_init(dwmac);
>> + if (ret)
>> + return ret;
>> +
>> + ret = stmmac_resume(dev);
>> +
>> + return ret;
>> +}
>> +#endif /* CONFIG_PM_SLEEP */
>
> With these changes:
> Acked-by: Joachim Eastwood <manabian@gmail.com>
>
>
> best regards,
> Joachim Eastwood
>
^ permalink raw reply
* [PATCH net-next v6 3/3] udp: use it's own memory accounting schema
From: Paolo Abeni @ 2016-10-21 11:55 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA
Cc: David S. Miller, James Morris, Trond Myklebust, Alexander Duyck,
Daniel Borkmann, Eric Dumazet, Tom Herbert, Hannes Frederic Sowa,
Edward Cree, linux-nfs-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <cover.1477043395.git.pabeni-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Completely avoid default sock memory accounting and replace it
with udp-specific accounting.
Since the new memory accounting model encapsulates completely
the required locking, remove the socket lock on both enqueue and
dequeue, and avoid using the backlog on enqueue.
Be sure to clean-up rx queue memory on socket destruction, using
udp its own sk_destruct.
Tested using pktgen with random src port, 64 bytes packet,
wire-speed on a 10G link as sender and udp_sink as the receiver,
using an l4 tuple rxhash to stress the contention, and one or more
udp_sink instances with reuseport.
nr readers Kpps (vanilla) Kpps (patched)
1 170 440
3 1250 2150
6 3000 3650
9 4200 4450
12 5700 6250
v4 -> v5:
- avoid unneeded test in first_packet_length
v3 -> v4:
- remove useless sk_rcvqueues_full() call
v2 -> v3:
- do not set the now unsed backlog_rcv callback
v1 -> v2:
- add memory pressure support
- fixed dropwatch accounting for ipv6
Acked-by: Hannes Frederic Sowa <hannes-tFNcAqjVMyqKXQKiL6tip0B+6BGkLq7r@public.gmane.org>
Signed-off-by: Paolo Abeni <pabeni-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
net/ipv4/udp.c | 43 ++++++++-----------------------------------
net/ipv6/udp.c | 34 ++++++++--------------------------
net/sunrpc/svcsock.c | 20 ++++++++++++++++----
net/sunrpc/xprtsock.c | 2 +-
4 files changed, 33 insertions(+), 66 deletions(-)
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index e137e1d..b38269d 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1307,13 +1307,7 @@ static int first_packet_length(struct sock *sk)
res = skb ? skb->len : -1;
spin_unlock_bh(&rcvq->lock);
- if (!skb_queue_empty(&list_kill)) {
- bool slow = lock_sock_fast(sk);
-
- __skb_queue_purge(&list_kill);
- sk_mem_reclaim_partial(sk);
- unlock_sock_fast(sk, slow);
- }
+ __skb_queue_purge(&list_kill);
return res;
}
@@ -1362,7 +1356,6 @@ int udp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, int noblock,
int err;
int is_udplite = IS_UDPLITE(sk);
bool checksum_valid = false;
- bool slow;
if (flags & MSG_ERRQUEUE)
return ip_recv_error(sk, msg, len, addr_len);
@@ -1403,13 +1396,12 @@ int udp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, int noblock,
}
if (unlikely(err)) {
- trace_kfree_skb(skb, udp_recvmsg);
if (!peeked) {
atomic_inc(&sk->sk_drops);
UDP_INC_STATS(sock_net(sk),
UDP_MIB_INERRORS, is_udplite);
}
- skb_free_datagram_locked(sk, skb);
+ kfree_skb(skb);
return err;
}
@@ -1434,16 +1426,15 @@ int udp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, int noblock,
if (flags & MSG_TRUNC)
err = ulen;
- __skb_free_datagram_locked(sk, skb, peeking ? -err : err);
+ skb_consume_udp(sk, skb, peeking ? -err : err);
return err;
csum_copy_err:
- slow = lock_sock_fast(sk);
- if (!skb_kill_datagram(sk, skb, flags)) {
+ if (!__sk_queue_drop_skb(sk, skb, flags)) {
UDP_INC_STATS(sock_net(sk), UDP_MIB_CSUMERRORS, is_udplite);
UDP_INC_STATS(sock_net(sk), UDP_MIB_INERRORS, is_udplite);
}
- unlock_sock_fast(sk, slow);
+ kfree_skb(skb);
/* starting over for a new packet, but check if we need to yield */
cond_resched();
@@ -1562,7 +1553,7 @@ static int __udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
sk_incoming_cpu_update(sk);
}
- rc = __sock_queue_rcv_skb(sk, skb);
+ rc = __udp_enqueue_schedule_skb(sk, skb);
if (rc < 0) {
int is_udplite = IS_UDPLITE(sk);
@@ -1577,7 +1568,6 @@ static int __udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
}
return 0;
-
}
static struct static_key udp_encap_needed __read_mostly;
@@ -1599,7 +1589,6 @@ void udp_encap_enable(void)
int udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
{
struct udp_sock *up = udp_sk(sk);
- int rc;
int is_udplite = IS_UDPLITE(sk);
/*
@@ -1686,25 +1675,9 @@ int udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
goto drop;
udp_csum_pull_header(skb);
- if (sk_rcvqueues_full(sk, sk->sk_rcvbuf)) {
- __UDP_INC_STATS(sock_net(sk), UDP_MIB_RCVBUFERRORS,
- is_udplite);
- goto drop;
- }
-
- rc = 0;
ipv4_pktinfo_prepare(sk, skb);
- bh_lock_sock(sk);
- if (!sock_owned_by_user(sk))
- rc = __udp_queue_rcv_skb(sk, skb);
- else if (sk_add_backlog(sk, skb, sk->sk_rcvbuf)) {
- bh_unlock_sock(sk);
- goto drop;
- }
- bh_unlock_sock(sk);
-
- return rc;
+ return __udp_queue_rcv_skb(sk, skb);
csum_error:
__UDP_INC_STATS(sock_net(sk), UDP_MIB_CSUMERRORS, is_udplite);
@@ -2314,13 +2287,13 @@ struct proto udp_prot = {
.connect = ip4_datagram_connect,
.disconnect = udp_disconnect,
.ioctl = udp_ioctl,
+ .init = udp_init_sock,
.destroy = udp_destroy_sock,
.setsockopt = udp_setsockopt,
.getsockopt = udp_getsockopt,
.sendmsg = udp_sendmsg,
.recvmsg = udp_recvmsg,
.sendpage = udp_sendpage,
- .backlog_rcv = __udp_queue_rcv_skb,
.release_cb = ip4_datagram_release_cb,
.hash = udp_lib_hash,
.unhash = udp_lib_unhash,
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 9aa7c1c..71963b2 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -334,7 +334,6 @@ int udpv6_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
int is_udplite = IS_UDPLITE(sk);
bool checksum_valid = false;
int is_udp4;
- bool slow;
if (flags & MSG_ERRQUEUE)
return ipv6_recv_error(sk, msg, len, addr_len);
@@ -378,7 +377,6 @@ int udpv6_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
goto csum_copy_err;
}
if (unlikely(err)) {
- trace_kfree_skb(skb, udpv6_recvmsg);
if (!peeked) {
atomic_inc(&sk->sk_drops);
if (is_udp4)
@@ -388,7 +386,7 @@ int udpv6_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
UDP6_INC_STATS(sock_net(sk), UDP_MIB_INERRORS,
is_udplite);
}
- skb_free_datagram_locked(sk, skb);
+ kfree_skb(skb);
return err;
}
if (!peeked) {
@@ -437,12 +435,11 @@ int udpv6_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
if (flags & MSG_TRUNC)
err = ulen;
- __skb_free_datagram_locked(sk, skb, peeking ? -err : err);
+ skb_consume_udp(sk, skb, peeking ? -err : err);
return err;
csum_copy_err:
- slow = lock_sock_fast(sk);
- if (!skb_kill_datagram(sk, skb, flags)) {
+ if (!__sk_queue_drop_skb(sk, skb, flags)) {
if (is_udp4) {
UDP_INC_STATS(sock_net(sk),
UDP_MIB_CSUMERRORS, is_udplite);
@@ -455,7 +452,7 @@ int udpv6_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
UDP_MIB_INERRORS, is_udplite);
}
}
- unlock_sock_fast(sk, slow);
+ kfree_skb(skb);
/* starting over for a new packet, but check if we need to yield */
cond_resched();
@@ -523,7 +520,7 @@ static int __udpv6_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
sk_incoming_cpu_update(sk);
}
- rc = __sock_queue_rcv_skb(sk, skb);
+ rc = __udp_enqueue_schedule_skb(sk, skb);
if (rc < 0) {
int is_udplite = IS_UDPLITE(sk);
@@ -535,6 +532,7 @@ static int __udpv6_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
kfree_skb(skb);
return -1;
}
+
return 0;
}
@@ -556,7 +554,6 @@ void udpv6_encap_enable(void)
int udpv6_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
{
struct udp_sock *up = udp_sk(sk);
- int rc;
int is_udplite = IS_UDPLITE(sk);
if (!xfrm6_policy_check(sk, XFRM_POLICY_IN, skb))
@@ -622,25 +619,10 @@ int udpv6_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
goto drop;
udp_csum_pull_header(skb);
- if (sk_rcvqueues_full(sk, sk->sk_rcvbuf)) {
- __UDP6_INC_STATS(sock_net(sk),
- UDP_MIB_RCVBUFERRORS, is_udplite);
- goto drop;
- }
skb_dst_drop(skb);
- bh_lock_sock(sk);
- rc = 0;
- if (!sock_owned_by_user(sk))
- rc = __udpv6_queue_rcv_skb(sk, skb);
- else if (sk_add_backlog(sk, skb, sk->sk_rcvbuf)) {
- bh_unlock_sock(sk);
- goto drop;
- }
- bh_unlock_sock(sk);
-
- return rc;
+ return __udpv6_queue_rcv_skb(sk, skb);
csum_error:
__UDP6_INC_STATS(sock_net(sk), UDP_MIB_CSUMERRORS, is_udplite);
@@ -1433,12 +1415,12 @@ struct proto udpv6_prot = {
.connect = ip6_datagram_connect,
.disconnect = udp_disconnect,
.ioctl = udp_ioctl,
+ .init = udp_init_sock,
.destroy = udpv6_destroy_sock,
.setsockopt = udpv6_setsockopt,
.getsockopt = udpv6_getsockopt,
.sendmsg = udpv6_sendmsg,
.recvmsg = udpv6_recvmsg,
- .backlog_rcv = __udpv6_queue_rcv_skb,
.release_cb = ip6_datagram_release_cb,
.hash = udp_lib_hash,
.unhash = udp_lib_unhash,
diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index 57625f6..e2a55dc 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -39,6 +39,7 @@
#include <net/checksum.h>
#include <net/ip.h>
#include <net/ipv6.h>
+#include <net/udp.h>
#include <net/tcp.h>
#include <net/tcp_states.h>
#include <asm/uaccess.h>
@@ -129,6 +130,18 @@ static void svc_release_skb(struct svc_rqst *rqstp)
}
}
+static void svc_release_udp_skb(struct svc_rqst *rqstp)
+{
+ struct sk_buff *skb = rqstp->rq_xprt_ctxt;
+
+ if (skb) {
+ rqstp->rq_xprt_ctxt = NULL;
+
+ dprintk("svc: service %p, releasing skb %p\n", rqstp, skb);
+ consume_skb(skb);
+ }
+}
+
union svc_pktinfo_u {
struct in_pktinfo pkti;
struct in6_pktinfo pkti6;
@@ -575,7 +588,7 @@ static int svc_udp_recvfrom(struct svc_rqst *rqstp)
goto out_free;
}
local_bh_enable();
- skb_free_datagram_locked(svsk->sk_sk, skb);
+ consume_skb(skb);
} else {
/* we can use it in-place */
rqstp->rq_arg.head[0].iov_base = skb->data;
@@ -602,8 +615,7 @@ static int svc_udp_recvfrom(struct svc_rqst *rqstp)
return len;
out_free:
- trace_kfree_skb(skb, svc_udp_recvfrom);
- skb_free_datagram_locked(svsk->sk_sk, skb);
+ kfree_skb(skb);
return 0;
}
@@ -660,7 +672,7 @@ static struct svc_xprt *svc_udp_create(struct svc_serv *serv,
.xpo_create = svc_udp_create,
.xpo_recvfrom = svc_udp_recvfrom,
.xpo_sendto = svc_udp_sendto,
- .xpo_release_rqst = svc_release_skb,
+ .xpo_release_rqst = svc_release_udp_skb,
.xpo_detach = svc_sock_detach,
.xpo_free = svc_sock_free,
.xpo_prep_reply_hdr = svc_udp_prep_reply_hdr,
diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index 0137af1..1758665 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -1083,7 +1083,7 @@ static void xs_udp_data_receive(struct sock_xprt *transport)
skb = skb_recv_datagram(sk, 0, 1, &err);
if (skb != NULL) {
xs_udp_data_read_skb(&transport->xprt, sk, skb);
- skb_free_datagram_locked(sk, skb);
+ consume_skb(skb);
continue;
}
if (!test_and_clear_bit(XPRT_SOCK_DATA_READY, &transport->sock_state))
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH net-next v6 0/3] udp: refactor memory accounting
From: Paolo Abeni @ 2016-10-21 11:55 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, James Morris, Trond Myklebust, Alexander Duyck,
Daniel Borkmann, Eric Dumazet, Tom Herbert, Hannes Frederic Sowa,
Edward Cree, linux-nfs
This patch series refactor the udp memory accounting, replacing the
generic implementation with a custom one, in order to remove the needs for
locking the socket on the enqueue and dequeue operations. The socket backlog
usage is dropped, as well.
The first patch factor out pieces of some queue and memory management
socket helpers, so that they can later be used by the udp memory accounting
functions.
The second patch adds the memory account helpers, without using them.
The third patch replacse the old rx memory accounting path for udp over ipv4 and
udp over ipv6. In kernel UDP users are updated, as well.
The memory accounting schema is described in detail in the individual patch
commit message.
The performance gain depends on the specific scenario; with few flows (and
little contention in the original code) the differences are in the noise range,
while with several flows contending the same socket, the measured speed-up
is relevant (e.g. even over 100% in case of extreme contention)
Many thanks to Eric Dumazet for the reiterated reviews and suggestions.
v5 -> v6:
- do not orphan the skb on enqueue, skb_steal_sock() already did
the work for us
v4 -> v5:
- use the receive queue spin lock to protect the memory accounting
- several minor clean-up
v3 -> v4:
- simplified the locking schema, always use a plain spinlock
v2 -> v3:
- do not set the now unsed backlog_rcv callback
v1 -> v2:
- changed slighly the memory accounting schema, we now perform lazy reclaim
- fixed forward_alloc updating issue
- fixed memory counter integer overflows
Paolo Abeni (3):
net/socket: factor out helpers for memory and queue manipulation
udp: implement memory accounting helpers
udp: use it's own memory accounting schema
include/net/sock.h | 4 ++
include/net/udp.h | 4 ++
net/core/datagram.c | 36 +++++++-----
net/core/sock.c | 64 +++++++++++++++-------
net/ipv4/udp.c | 149 ++++++++++++++++++++++++++++++++++++++------------
net/ipv6/udp.c | 34 +++---------
net/sunrpc/svcsock.c | 20 +++++--
net/sunrpc/xprtsock.c | 2 +-
8 files changed, 214 insertions(+), 99 deletions(-)
--
1.8.3.1
^ permalink raw reply
* [PATCH net-next v6 1/3] net/socket: factor out helpers for memory and queue manipulation
From: Paolo Abeni @ 2016-10-21 11:55 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, James Morris, Trond Myklebust, Alexander Duyck,
Daniel Borkmann, Eric Dumazet, Tom Herbert, Hannes Frederic Sowa,
Edward Cree, linux-nfs
In-Reply-To: <cover.1477043395.git.pabeni@redhat.com>
Basic sock operations that udp code can use with its own
memory accounting schema. No functional change is introduced
in the existing APIs.
v4 -> v5:
- avoid whitespace changes
v2 -> v4:
- avoid exporting __sock_enqueue_skb
v1 -> v2:
- avoid export sock_rmem_free
Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
include/net/sock.h | 4 ++++
net/core/datagram.c | 36 ++++++++++++++++++------------
net/core/sock.c | 64 +++++++++++++++++++++++++++++++++++++----------------
3 files changed, 71 insertions(+), 33 deletions(-)
diff --git a/include/net/sock.h b/include/net/sock.h
index ebf75db..2764895 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1274,7 +1274,9 @@ static inline struct inode *SOCK_INODE(struct socket *socket)
/*
* Functions for memory accounting
*/
+int __sk_mem_raise_allocated(struct sock *sk, int size, int amt, int kind);
int __sk_mem_schedule(struct sock *sk, int size, int kind);
+void __sk_mem_reduce_allocated(struct sock *sk, int amount);
void __sk_mem_reclaim(struct sock *sk, int amount);
#define SK_MEM_QUANTUM ((int)PAGE_SIZE)
@@ -1950,6 +1952,8 @@ void sk_reset_timer(struct sock *sk, struct timer_list *timer,
void sk_stop_timer(struct sock *sk, struct timer_list *timer);
+int __sk_queue_drop_skb(struct sock *sk, struct sk_buff *skb,
+ unsigned int flags);
int __sock_queue_rcv_skb(struct sock *sk, struct sk_buff *skb);
int sock_queue_rcv_skb(struct sock *sk, struct sk_buff *skb);
diff --git a/net/core/datagram.c b/net/core/datagram.c
index b7de71f..bfb973a 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -323,6 +323,27 @@ void __skb_free_datagram_locked(struct sock *sk, struct sk_buff *skb, int len)
}
EXPORT_SYMBOL(__skb_free_datagram_locked);
+int __sk_queue_drop_skb(struct sock *sk, struct sk_buff *skb,
+ unsigned int flags)
+{
+ int err = 0;
+
+ if (flags & MSG_PEEK) {
+ err = -ENOENT;
+ spin_lock_bh(&sk->sk_receive_queue.lock);
+ if (skb == skb_peek(&sk->sk_receive_queue)) {
+ __skb_unlink(skb, &sk->sk_receive_queue);
+ atomic_dec(&skb->users);
+ err = 0;
+ }
+ spin_unlock_bh(&sk->sk_receive_queue.lock);
+ }
+
+ atomic_inc(&sk->sk_drops);
+ return err;
+}
+EXPORT_SYMBOL(__sk_queue_drop_skb);
+
/**
* skb_kill_datagram - Free a datagram skbuff forcibly
* @sk: socket
@@ -346,23 +367,10 @@ void __skb_free_datagram_locked(struct sock *sk, struct sk_buff *skb, int len)
int skb_kill_datagram(struct sock *sk, struct sk_buff *skb, unsigned int flags)
{
- int err = 0;
-
- if (flags & MSG_PEEK) {
- err = -ENOENT;
- spin_lock_bh(&sk->sk_receive_queue.lock);
- if (skb == skb_peek(&sk->sk_receive_queue)) {
- __skb_unlink(skb, &sk->sk_receive_queue);
- atomic_dec(&skb->users);
- err = 0;
- }
- spin_unlock_bh(&sk->sk_receive_queue.lock);
- }
+ int err = __sk_queue_drop_skb(sk, skb, flags);
kfree_skb(skb);
- atomic_inc(&sk->sk_drops);
sk_mem_reclaim_partial(sk);
-
return err;
}
EXPORT_SYMBOL(skb_kill_datagram);
diff --git a/net/core/sock.c b/net/core/sock.c
index c73e28f..d8e4532e 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -2091,24 +2091,18 @@ int sk_wait_data(struct sock *sk, long *timeo, const struct sk_buff *skb)
EXPORT_SYMBOL(sk_wait_data);
/**
- * __sk_mem_schedule - increase sk_forward_alloc and memory_allocated
+ * __sk_mem_raise_allocated - increase memory_allocated
* @sk: socket
* @size: memory size to allocate
+ * @amt: pages to allocate
* @kind: allocation type
*
- * If kind is SK_MEM_SEND, it means wmem allocation. Otherwise it means
- * rmem allocation. This function assumes that protocols which have
- * memory_pressure use sk_wmem_queued as write buffer accounting.
+ * Similar to __sk_mem_schedule(), but does not update sk_forward_alloc
*/
-int __sk_mem_schedule(struct sock *sk, int size, int kind)
+int __sk_mem_raise_allocated(struct sock *sk, int size, int amt, int kind)
{
struct proto *prot = sk->sk_prot;
- int amt = sk_mem_pages(size);
- long allocated;
-
- sk->sk_forward_alloc += amt * SK_MEM_QUANTUM;
-
- allocated = sk_memory_allocated_add(sk, amt);
+ long allocated = sk_memory_allocated_add(sk, amt);
if (mem_cgroup_sockets_enabled && sk->sk_memcg &&
!mem_cgroup_charge_skmem(sk->sk_memcg, amt))
@@ -2169,9 +2163,6 @@ int __sk_mem_schedule(struct sock *sk, int size, int kind)
trace_sock_exceed_buf_limit(sk, prot, allocated);
- /* Alas. Undo changes. */
- sk->sk_forward_alloc -= amt * SK_MEM_QUANTUM;
-
sk_memory_allocated_sub(sk, amt);
if (mem_cgroup_sockets_enabled && sk->sk_memcg)
@@ -2179,18 +2170,40 @@ int __sk_mem_schedule(struct sock *sk, int size, int kind)
return 0;
}
+EXPORT_SYMBOL(__sk_mem_raise_allocated);
+
+/**
+ * __sk_mem_schedule - increase sk_forward_alloc and memory_allocated
+ * @sk: socket
+ * @size: memory size to allocate
+ * @kind: allocation type
+ *
+ * If kind is SK_MEM_SEND, it means wmem allocation. Otherwise it means
+ * rmem allocation. This function assumes that protocols which have
+ * memory_pressure use sk_wmem_queued as write buffer accounting.
+ */
+int __sk_mem_schedule(struct sock *sk, int size, int kind)
+{
+ int ret, amt = sk_mem_pages(size);
+
+ sk->sk_forward_alloc += amt << SK_MEM_QUANTUM_SHIFT;
+ ret = __sk_mem_raise_allocated(sk, size, amt, kind);
+ if (!ret)
+ sk->sk_forward_alloc -= amt << SK_MEM_QUANTUM_SHIFT;
+ return ret;
+}
EXPORT_SYMBOL(__sk_mem_schedule);
/**
- * __sk_mem_reclaim - reclaim memory_allocated
+ * __sk_mem_reduce_allocated - reclaim memory_allocated
* @sk: socket
- * @amount: number of bytes (rounded down to a SK_MEM_QUANTUM multiple)
+ * @amount: number of quanta
+ *
+ * Similar to __sk_mem_reclaim(), but does not update sk_forward_alloc
*/
-void __sk_mem_reclaim(struct sock *sk, int amount)
+void __sk_mem_reduce_allocated(struct sock *sk, int amount)
{
- amount >>= SK_MEM_QUANTUM_SHIFT;
sk_memory_allocated_sub(sk, amount);
- sk->sk_forward_alloc -= amount << SK_MEM_QUANTUM_SHIFT;
if (mem_cgroup_sockets_enabled && sk->sk_memcg)
mem_cgroup_uncharge_skmem(sk->sk_memcg, amount);
@@ -2199,6 +2212,19 @@ void __sk_mem_reclaim(struct sock *sk, int amount)
(sk_memory_allocated(sk) < sk_prot_mem_limits(sk, 0)))
sk_leave_memory_pressure(sk);
}
+EXPORT_SYMBOL(__sk_mem_reduce_allocated);
+
+/**
+ * __sk_mem_reclaim - reclaim sk_forward_alloc and memory_allocated
+ * @sk: socket
+ * @amount: number of bytes (rounded down to a SK_MEM_QUANTUM multiple)
+ */
+void __sk_mem_reclaim(struct sock *sk, int amount)
+{
+ amount >>= SK_MEM_QUANTUM_SHIFT;
+ sk->sk_forward_alloc -= amount << SK_MEM_QUANTUM_SHIFT;
+ __sk_mem_reduce_allocated(sk, amount);
+}
EXPORT_SYMBOL(__sk_mem_reclaim);
int sk_set_peek_off(struct sock *sk, int val)
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next v6 2/3] udp: implement memory accounting helpers
From: Paolo Abeni @ 2016-10-21 11:55 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, James Morris, Trond Myklebust, Alexander Duyck,
Daniel Borkmann, Eric Dumazet, Tom Herbert, Hannes Frederic Sowa,
Edward Cree, linux-nfs
In-Reply-To: <cover.1477043395.git.pabeni@redhat.com>
Avoid using the generic helpers.
Use the receive queue spin lock to protect the memory
accounting operation, both on enqueue and on dequeue.
On dequeue perform partial memory reclaiming, trying to
leave a quantum of forward allocated memory.
On enqueue use a custom helper, to allow some optimizations:
- use a plain spin_lock() variant instead of the slightly
costly spin_lock_irqsave(),
- avoid dst_force check, since the calling code has already
dropped the skb dst
- avoid orphaning the skb, since skb_steal_sock() already did
the work for us
The above needs custom memory reclaiming on shutdown, provided
by the udp_destruct_sock().
v5 -> v6:
- don't orphan the skb on enqueue
v4 -> v5:
- replace the mem_lock with the receive queue spin lock
- ensure that the bh is always allowed to enqueue at least
a skb, even if sk_rcvbuf is exceeded
v3 -> v4:
- reworked memory accunting, simplifying the schema
- provide an helper for both memory scheduling and enqueuing
v1 -> v2:
- use a udp specific destrctor to perform memory reclaiming
- remove a couple of helpers, unneeded after the above cleanup
- do not reclaim memory on dequeue if not under memory
pressure
- reworked the fwd accounting schema to avoid potential
integer overflow
Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
include/net/udp.h | 4 +++
net/ipv4/udp.c | 106 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 110 insertions(+)
diff --git a/include/net/udp.h b/include/net/udp.h
index ea53a87..18f1e6b 100644
--- a/include/net/udp.h
+++ b/include/net/udp.h
@@ -246,6 +246,9 @@ static inline __be16 udp_flow_src_port(struct net *net, struct sk_buff *skb,
}
/* net/ipv4/udp.c */
+void skb_consume_udp(struct sock *sk, struct sk_buff *skb, int len);
+int __udp_enqueue_schedule_skb(struct sock *sk, struct sk_buff *skb);
+
void udp_v4_early_demux(struct sk_buff *skb);
int udp_get_port(struct sock *sk, unsigned short snum,
int (*saddr_cmp)(const struct sock *,
@@ -258,6 +261,7 @@ int udp_get_port(struct sock *sk, unsigned short snum,
void udp4_hwcsum(struct sk_buff *skb, __be32 src, __be32 dst);
int udp_rcv(struct sk_buff *skb);
int udp_ioctl(struct sock *sk, int cmd, unsigned long arg);
+int udp_init_sock(struct sock *sk);
int udp_disconnect(struct sock *sk, int flags);
unsigned int udp_poll(struct file *file, struct socket *sock, poll_table *wait);
struct sk_buff *skb_udp_tunnel_segment(struct sk_buff *skb,
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 7d96dc2..e137e1d 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1172,6 +1172,112 @@ int udp_sendpage(struct sock *sk, struct page *page, int offset,
return ret;
}
+static void udp_rmem_release(struct sock *sk, int size, int partial)
+{
+ int amt;
+
+ atomic_sub(size, &sk->sk_rmem_alloc);
+
+ spin_lock_bh(&sk->sk_receive_queue.lock);
+ sk->sk_forward_alloc += size;
+ amt = (sk->sk_forward_alloc - partial) & ~(SK_MEM_QUANTUM - 1);
+ sk->sk_forward_alloc -= amt;
+ spin_unlock_bh(&sk->sk_receive_queue.lock);
+
+ if (amt)
+ __sk_mem_reduce_allocated(sk, amt >> SK_MEM_QUANTUM_SHIFT);
+}
+
+static void udp_rmem_free(struct sk_buff *skb)
+{
+ udp_rmem_release(skb->sk, skb->truesize, 1);
+}
+
+int __udp_enqueue_schedule_skb(struct sock *sk, struct sk_buff *skb)
+{
+ struct sk_buff_head *list = &sk->sk_receive_queue;
+ int rmem, delta, amt, err = -ENOMEM;
+ int size = skb->truesize;
+
+ /* try to avoid the costly atomic add/sub pair when the receive
+ * queue is full; always allow at least a packet
+ */
+ rmem = atomic_read(&sk->sk_rmem_alloc);
+ if (rmem && (rmem + size > sk->sk_rcvbuf))
+ goto drop;
+
+ /* we drop only if the receive buf is full and the receive
+ * queue contains some other skb
+ */
+ rmem = atomic_add_return(size, &sk->sk_rmem_alloc);
+ if ((rmem > sk->sk_rcvbuf) && (rmem > size))
+ goto uncharge_drop;
+
+ spin_lock(&list->lock);
+ if (size >= sk->sk_forward_alloc) {
+ amt = sk_mem_pages(size);
+ delta = amt << SK_MEM_QUANTUM_SHIFT;
+ if (!__sk_mem_raise_allocated(sk, delta, amt, SK_MEM_RECV)) {
+ err = -ENOBUFS;
+ spin_unlock(&list->lock);
+ goto uncharge_drop;
+ }
+
+ sk->sk_forward_alloc += delta;
+ }
+
+ sk->sk_forward_alloc -= size;
+
+ /* the skb owner in now the udp socket */
+ skb->sk = sk;
+ skb->destructor = udp_rmem_free;
+ skb->dev = NULL;
+ sock_skb_set_dropcount(sk, skb);
+
+ __skb_queue_tail(list, skb);
+ spin_unlock(&list->lock);
+
+ if (!sock_flag(sk, SOCK_DEAD))
+ sk->sk_data_ready(sk);
+
+ return 0;
+
+uncharge_drop:
+ atomic_sub(skb->truesize, &sk->sk_rmem_alloc);
+
+drop:
+ atomic_inc(&sk->sk_drops);
+ return err;
+}
+EXPORT_SYMBOL_GPL(__udp_enqueue_schedule_skb);
+
+static void udp_destruct_sock(struct sock *sk)
+{
+ /* reclaim completely the forward allocated memory */
+ __skb_queue_purge(&sk->sk_receive_queue);
+ udp_rmem_release(sk, 0, 0);
+ inet_sock_destruct(sk);
+}
+
+int udp_init_sock(struct sock *sk)
+{
+ sk->sk_destruct = udp_destruct_sock;
+ return 0;
+}
+EXPORT_SYMBOL_GPL(udp_init_sock);
+
+void skb_consume_udp(struct sock *sk, struct sk_buff *skb, int len)
+{
+ if (unlikely(READ_ONCE(sk->sk_peek_off) >= 0)) {
+ bool slow = lock_sock_fast(sk);
+
+ sk_peek_offset_bwd(sk, len);
+ unlock_sock_fast(sk, slow);
+ }
+ consume_skb(skb);
+}
+EXPORT_SYMBOL_GPL(skb_consume_udp);
+
/**
* first_packet_length - return length of first packet in receive queue
* @sk: socket
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH net-next v12 5/9] openvswitch: add processing of L3 packets
From: Jiri Benc @ 2016-10-21 11:59 UTC (permalink / raw)
To: Pravin Shelar
Cc: Linux Kernel Network Developers, ovs dev, Lorand Jakab,
Simon Horman
In-Reply-To: <CAOrHB_Co3DaYJPbrcDRrnQ1P18X9B=n867uWEiaFqJcKPHs=2A@mail.gmail.com>
On Thu, 20 Oct 2016 21:19:14 -0700, Pravin Shelar wrote:
> On Mon, Oct 17, 2016 at 6:02 AM, Jiri Benc <jbenc@redhat.com> wrote:
> > @@ -609,8 +597,10 @@ static int ovs_packet_cmd_execute(struct sk_buff *skb, struct genl_info *info)
> >
> > err = ovs_flow_key_extract_userspace(net, a[OVS_PACKET_ATTR_KEY],
> > packet, &flow->key, log);
> > - if (err)
> > + if (err) {
> > + packet = NULL;
> > goto err_flow_free;
> > + }
> >
> Why packet is set to NULL? This would leak skb memory in case of error.
Leftover from the times when this was based on top of the patchset that
unified vlan handling. Thanks for catching it.
> > +static u8 key_extract_mac_proto(struct sk_buff *skb)
> > +{
> > + switch (skb->dev->type) {
> > + case ARPHRD_ETHER:
> > + return MAC_PROTO_ETHERNET;
> > + case ARPHRD_NONE:
> > + if (skb->protocol == htons(ETH_P_TEB))
> > + return MAC_PROTO_ETHERNET;
> > + return MAC_PROTO_NONE;
> > + }
> > + WARN_ON_ONCE(1);
> > + return MAC_PROTO_ETHERNET;
> This packet should be dropped.
This should never happen, I put WARN_ON_ONCE there to catch impossible
to happen bugs (and BUG_ON is really too strong here). But sure, why
not, will drop the packet in v13.
> > + switch (vport->dev->type) {
> > + case ARPHRD_NONE:
> > + if (mac_proto != MAC_PROTO_NONE) {
> > + WARN_ON_ONCE(mac_proto != MAC_PROTO_ETHERNET);
> > +
> It would be easy to read if you check mac_proto for MAC_PROTO_ETHERNET
> and then warn otherwise.
Okay.
> another issue is the packet is not dropped if mac_proto is not
> MAC_PROTO_ETHERNET
Likewise, impossible to happen, but will drop the packet.
Thanks,
Jiri
^ permalink raw reply
* Re: [PATCH net-next 3/6] net: use core MTU range checking in WAN drivers
From: Krzysztof Hałasa @ 2016-10-21 12:04 UTC (permalink / raw)
To: Jarod Wilson
Cc: linux-kernel, netdev, Krzysztof Halasa, Jan Yenya Kasprzak,
Francois Romieu, Kevin Curtis, Zhao Qiang
In-Reply-To: <20161019023333.15760-4-jarod@redhat.com>
Jarod Wilson <jarod@redhat.com> writes:
> - set min/max_mtu in all hdlc drivers, remove hdlc_change_mtu
> - sent max_mtu in lec driver, remove lec_change_mtu
> drivers/net/wan/c101.c | 1 -
> drivers/net/wan/hdlc.c | 11 ++---------
> drivers/net/wan/hdlc_fr.c | 3 ++-
> drivers/net/wan/ixp4xx_hss.c | 1 -
> drivers/net/wan/n2.c | 1 -
> drivers/net/wan/pc300too.c | 1 -
> drivers/net/wan/pci200syn.c | 1 -
> drivers/net/wan/wanxl.c | 1 -
> include/linux/hdlc.h | 2 --
Acked-by: Krzysztof Halasa <khalasa@piap.pl>
--
Krzysztof Halasa
Industrial Research Institute for Automation and Measurements PIAP
Al. Jerozolimskie 202, 02-486 Warsaw, Poland
^ permalink raw reply
* [PATCH] net: sctp, forbid negative length
From: Jiri Slaby @ 2016-10-21 12:13 UTC (permalink / raw)
To: Vlad Yasevich
Cc: linux-kernel, Jiri Slaby, Neil Horman, David S. Miller,
linux-sctp, netdev
Most of getsockopt handlers in net/sctp/socket.c check len against
sizeof some structure like:
if (len < sizeof(int))
return -EINVAL;
On the first look, the check seems to be correct. But since len is int
and sizeof returns size_t, int gets promoted to unsigned size_t too. So
the test returns false for negative lengths. Yes, (-1 < sizeof(long)) is
false.
Fix this in sctp by explicitly checking len < 0 before any getsockopt
handler is called.
Note that sctp_getsockopt_events already handled the negative case.
Since we added the < 0 check elsewhere, this one can be removed.
If not checked, this is the result:
UBSAN: Undefined behaviour in ../mm/page_alloc.c:2722:19
shift exponent 52 is too large for 32-bit type 'int'
CPU: 1 PID: 24535 Comm: syz-executor Not tainted 4.8.1-0-syzkaller #1
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.9.1-0-gb3ef39f-prebuilt.qemu-project.org 04/01/2014
0000000000000000 ffff88006d99f2a8 ffffffffb2f7bdea 0000000041b58ab3
ffffffffb4363c14 ffffffffb2f7bcde ffff88006d99f2d0 ffff88006d99f270
0000000000000000 0000000000000000 0000000000000034 ffffffffb5096422
Call Trace:
[<ffffffffb3051498>] ? __ubsan_handle_shift_out_of_bounds+0x29c/0x300
...
[<ffffffffb273f0e4>] ? kmalloc_order+0x24/0x90
[<ffffffffb27416a4>] ? kmalloc_order_trace+0x24/0x220
[<ffffffffb2819a30>] ? __kmalloc+0x330/0x540
[<ffffffffc18c25f4>] ? sctp_getsockopt_local_addrs+0x174/0xca0 [sctp]
[<ffffffffc18d2bcd>] ? sctp_getsockopt+0x10d/0x1b0 [sctp]
[<ffffffffb37c1219>] ? sock_common_getsockopt+0xb9/0x150
[<ffffffffb37be2f5>] ? SyS_getsockopt+0x1a5/0x270
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Vlad Yasevich <vyasevich@gmail.com>
Cc: Neil Horman <nhorman@tuxdriver.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: linux-sctp@vger.kernel.org
Cc: netdev@vger.kernel.org
---
net/sctp/socket.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index fb02c7033307..9fbb6feb8c27 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -4687,7 +4687,7 @@ static int sctp_getsockopt_disable_fragments(struct sock *sk, int len,
static int sctp_getsockopt_events(struct sock *sk, int len, char __user *optval,
int __user *optlen)
{
- if (len <= 0)
+ if (len == 0)
return -EINVAL;
if (len > sizeof(struct sctp_event_subscribe))
len = sizeof(struct sctp_event_subscribe);
@@ -6430,6 +6430,9 @@ static int sctp_getsockopt(struct sock *sk, int level, int optname,
if (get_user(len, optlen))
return -EFAULT;
+ if (len < 0)
+ return -EINVAL;
+
lock_sock(sk);
switch (optname) {
--
2.10.1
^ permalink raw reply related
* Re: [PATCH net-next v12 7/9] openvswitch: add Ethernet push and pop actions
From: Jiri Benc @ 2016-10-21 12:21 UTC (permalink / raw)
To: Pravin Shelar
Cc: Linux Kernel Network Developers, ovs dev, Lorand Jakab,
Simon Horman
In-Reply-To: <CAOrHB_Bj+0DNePDKQ64Gtx9SyxS4cy20Y-1RLC4Dpd3DKJpsjA@mail.gmail.com>
On Thu, 20 Oct 2016 21:22:21 -0700, Pravin Shelar wrote:
> The eth pop substracts ETH_HLEN but here the length is set. I think it
> should be consistent with respect to eth-pop.
Agreed. Will use skb_reset_mac_len in both, that better reflects what's
going on.
Thanks,
Jiri
^ permalink raw reply
* Re: [PATCH net-next v6 2/3] udp: implement memory accounting helpers
From: Eric Dumazet @ 2016-10-21 12:24 UTC (permalink / raw)
To: Paolo Abeni
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, David S. Miller, James Morris,
Trond Myklebust, Alexander Duyck, Daniel Borkmann, Eric Dumazet,
Tom Herbert, Hannes Frederic Sowa, Edward Cree,
linux-nfs-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4d2e0fc8f5c3d1309b0fb71bc65a2719a8e82825.1477043395.git.pabeni-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Fri, 2016-10-21 at 13:55 +0200, Paolo Abeni wrote:
> Avoid using the generic helpers.
> Use the receive queue spin lock to protect the memory
> accounting operation, both on enqueue and on dequeue.
>
> On dequeue perform partial memory reclaiming, trying to
> leave a quantum of forward allocated memory.
>
> On enqueue use a custom helper, to allow some optimizations:
> - use a plain spin_lock() variant instead of the slightly
> costly spin_lock_irqsave(),
> - avoid dst_force check, since the calling code has already
> dropped the skb dst
> - avoid orphaning the skb, since skb_steal_sock() already did
> the work for us
>
> +static void udp_rmem_release(struct sock *sk, int size, int partial)
> +{
> + int amt;
> +
> + atomic_sub(size, &sk->sk_rmem_alloc);
> +
> + spin_lock_bh(&sk->sk_receive_queue.lock);
> + sk->sk_forward_alloc += size;
> + amt = (sk->sk_forward_alloc - partial) & ~(SK_MEM_QUANTUM - 1);
> + sk->sk_forward_alloc -= amt;
> + spin_unlock_bh(&sk->sk_receive_queue.lock);
> +
> + if (amt)
> + __sk_mem_reduce_allocated(sk, amt >> SK_MEM_QUANTUM_SHIFT);
> +}
> +
> +static void udp_rmem_free(struct sk_buff *skb)
> +{
> + udp_rmem_release(skb->sk, skb->truesize, 1);
> +}
> +
It looks like you are acquiring/releasing sk_receive_queue.lock twice
per packet in recvmsg() (the second time in the destructor above)
We could do slightly better if :
We do not set skb->destructor at all, and manage
sk_rmem_alloc/sk_forward_alloc changes at the time we dequeue skb
(if !MSG_PEEK), before copy to user space.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] net: sctp, forbid negative length
From: Neil Horman @ 2016-10-21 12:39 UTC (permalink / raw)
To: Jiri Slaby
Cc: Vlad Yasevich, linux-kernel, David S. Miller, linux-sctp, netdev
In-Reply-To: <20161021121324.13942-1-jslaby@suse.cz>
On Fri, Oct 21, 2016 at 02:13:24PM +0200, Jiri Slaby wrote:
> Most of getsockopt handlers in net/sctp/socket.c check len against
> sizeof some structure like:
> if (len < sizeof(int))
> return -EINVAL;
>
> On the first look, the check seems to be correct. But since len is int
> and sizeof returns size_t, int gets promoted to unsigned size_t too. So
> the test returns false for negative lengths. Yes, (-1 < sizeof(long)) is
> false.
>
> Fix this in sctp by explicitly checking len < 0 before any getsockopt
> handler is called.
>
> Note that sctp_getsockopt_events already handled the negative case.
> Since we added the < 0 check elsewhere, this one can be removed.
>
> If not checked, this is the result:
> UBSAN: Undefined behaviour in ../mm/page_alloc.c:2722:19
> shift exponent 52 is too large for 32-bit type 'int'
> CPU: 1 PID: 24535 Comm: syz-executor Not tainted 4.8.1-0-syzkaller #1
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.9.1-0-gb3ef39f-prebuilt.qemu-project.org 04/01/2014
> 0000000000000000 ffff88006d99f2a8 ffffffffb2f7bdea 0000000041b58ab3
> ffffffffb4363c14 ffffffffb2f7bcde ffff88006d99f2d0 ffff88006d99f270
> 0000000000000000 0000000000000000 0000000000000034 ffffffffb5096422
> Call Trace:
> [<ffffffffb3051498>] ? __ubsan_handle_shift_out_of_bounds+0x29c/0x300
> ...
> [<ffffffffb273f0e4>] ? kmalloc_order+0x24/0x90
> [<ffffffffb27416a4>] ? kmalloc_order_trace+0x24/0x220
> [<ffffffffb2819a30>] ? __kmalloc+0x330/0x540
> [<ffffffffc18c25f4>] ? sctp_getsockopt_local_addrs+0x174/0xca0 [sctp]
> [<ffffffffc18d2bcd>] ? sctp_getsockopt+0x10d/0x1b0 [sctp]
> [<ffffffffb37c1219>] ? sock_common_getsockopt+0xb9/0x150
> [<ffffffffb37be2f5>] ? SyS_getsockopt+0x1a5/0x270
>
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> Cc: Vlad Yasevich <vyasevich@gmail.com>
> Cc: Neil Horman <nhorman@tuxdriver.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: linux-sctp@vger.kernel.org
> Cc: netdev@vger.kernel.org
> ---
> net/sctp/socket.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index fb02c7033307..9fbb6feb8c27 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -4687,7 +4687,7 @@ static int sctp_getsockopt_disable_fragments(struct sock *sk, int len,
> static int sctp_getsockopt_events(struct sock *sk, int len, char __user *optval,
> int __user *optlen)
> {
> - if (len <= 0)
> + if (len == 0)
> return -EINVAL;
> if (len > sizeof(struct sctp_event_subscribe))
> len = sizeof(struct sctp_event_subscribe);
> @@ -6430,6 +6430,9 @@ static int sctp_getsockopt(struct sock *sk, int level, int optname,
> if (get_user(len, optlen))
> return -EFAULT;
>
> + if (len < 0)
> + return -EINVAL;
> +
> lock_sock(sk);
>
> switch (optname) {
> --
> 2.10.1
>
>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
^ permalink raw reply
* Re: [PATCH] net: sched: make default fifo qdiscs appear in the dump
From: Jiri Kosina @ 2016-10-21 12:56 UTC (permalink / raw)
To: Eric Dumazet
Cc: Jamal Hadi Salim, Phil Sutter, Cong Wang, Daniel Borkmann,
David S. Miller, linux-kernel, netdev
In-Reply-To: <1477049817.7065.56.camel@edumazet-glaptop3.roam.corp.google.com>
On Fri, 21 Oct 2016, Eric Dumazet wrote:
> Some of us are dealing with huge HTB hierarchies, so adding default fifo
> in the dump will add more data pumped from the kernel.
>
> BwE [1] for instance dumps qdisc/classes every 5 seconds.
>
> I guess we'll need to not pull this patch in our kernels.
Okay, so I probably misunderstood you here:
https://marc.info/?l=linux-kernel&m=146073234818214&w=2
as I thought that as long as we move towards the hashtable, you wouldn't
have any issues with this.
I'd really like to unhide the default qdiscs, it makes little sense to be
inconsistent in this way.
Random shot into darkness -- how about making this a
CONFIG/sysctl-selectable?
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] net: sched: make default fifo qdiscs appear in the dump
From: Eric Dumazet @ 2016-10-21 13:14 UTC (permalink / raw)
To: Jiri Kosina
Cc: Jamal Hadi Salim, Phil Sutter, Cong Wang, Daniel Borkmann,
David S. Miller, linux-kernel, netdev
In-Reply-To: <alpine.LNX.2.00.1610211450440.31629@cbobk.fhfr.pm>
On Fri, 2016-10-21 at 14:56 +0200, Jiri Kosina wrote:
> On Fri, 21 Oct 2016, Eric Dumazet wrote:
>
> > Some of us are dealing with huge HTB hierarchies, so adding default fifo
> > in the dump will add more data pumped from the kernel.
> >
> > BwE [1] for instance dumps qdisc/classes every 5 seconds.
> >
> > I guess we'll need to not pull this patch in our kernels.
>
> Okay, so I probably misunderstood you here:
>
> https://marc.info/?l=linux-kernel&m=146073234818214&w=2
>
> as I thought that as long as we move towards the hashtable, you wouldn't
> have any issues with this.
>
> I'd really like to unhide the default qdiscs, it makes little sense to be
> inconsistent in this way.
>
> Random shot into darkness -- how about making this a
> CONFIG/sysctl-selectable?
Oh sorry for the confusion, I believe your patch is fine.
We could add an netlink attribute later for the users that really do not
want default fifo being dumped, but there is no hurry.
Acked-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* Re: [PATCH net-next v2 6/9] net: use core MTU range checking in virt drivers
From: Aaron Conole @ 2016-10-21 13:24 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Jarod Wilson, David Kershner, Wei Liu, VMware, Inc., netdev,
Haiyang Zhang, linux-kernel, virtualization, Paul Durrant,
Shrikrishna Khare
In-Reply-To: <20161021063505-mutt-send-email-mst@kernel.org>
"Michael S. Tsirkin" <mst@redhat.com> writes:
> On Thu, Oct 20, 2016 at 10:37:20PM -0400, Jarod Wilson wrote:
>> On Thu, Oct 20, 2016 at 11:23:54PM +0300, Michael S. Tsirkin wrote:
>> > On Thu, Oct 20, 2016 at 01:55:21PM -0400, Jarod Wilson wrote:
>> ...
>> > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> > > index fad84f3..720809f 100644
>> > > --- a/drivers/net/virtio_net.c
>> > > +++ b/drivers/net/virtio_net.c
>> > > @@ -1419,17 +1419,6 @@ static const struct ethtool_ops virtnet_ethtool_ops = {
>> > > .set_settings = virtnet_set_settings,
>> > > };
>> > >
>> > > -#define MIN_MTU 68
>> > > -#define MAX_MTU 65535
>> > > -
>> > > -static int virtnet_change_mtu(struct net_device *dev, int new_mtu)
>> > > -{
>> > > - if (new_mtu < MIN_MTU || new_mtu > MAX_MTU)
>> > > - return -EINVAL;
>> > > - dev->mtu = new_mtu;
>> > > - return 0;
>> > > -}
>> > > -
>> > > static const struct net_device_ops virtnet_netdev = {
>> > > .ndo_open = virtnet_open,
>> > > .ndo_stop = virtnet_close,
>> > > @@ -1437,7 +1426,6 @@ static const struct net_device_ops virtnet_netdev = {
>> > > .ndo_validate_addr = eth_validate_addr,
>> > > .ndo_set_mac_address = virtnet_set_mac_address,
>> > > .ndo_set_rx_mode = virtnet_set_rx_mode,
>> > > - .ndo_change_mtu = virtnet_change_mtu,
>> > > .ndo_get_stats64 = virtnet_stats,
>> > > .ndo_vlan_rx_add_vid = virtnet_vlan_rx_add_vid,
>> > > .ndo_vlan_rx_kill_vid = virtnet_vlan_rx_kill_vid,
>> > > @@ -1748,6 +1736,9 @@ static bool virtnet_validate_features(struct virtio_device *vdev)
>> > > return true;
>> > > }
>> > >
>> > > +#define MIN_MTU ETH_MIN_MTU
>> > > +#define MAX_MTU ETH_MAX_MTU
>> > > +
>> >
>> > Can we drop these btw?
>>
>> Bah. Yeah. Should have just used them directly. I didn't add ETH_MAX_MTU
>> until after doing the virtio_net changes, so I missed that.
>>
>> > > static int virtnet_probe(struct virtio_device *vdev)
>> > > {
>> > > int i, err;
>> > > @@ -1821,6 +1812,10 @@ static int virtnet_probe(struct virtio_device *vdev)
>> > >
>> > > dev->vlan_features = dev->features;
>> > >
>> > > + /* MTU range: 68 - 65535 */
>> > > + dev->min_mtu = MIN_MTU;
>> > > + dev->max_mtu = MAX_MTU;
>> > > +
>> > > /* Configuration may specify what MAC to use. Otherwise random. */
>> > > if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC))
>> > > virtio_cread_bytes(vdev,
>> > > @@ -1875,8 +1870,10 @@ static int virtnet_probe(struct virtio_device *vdev)
>> > > mtu = virtio_cread16(vdev,
>> > > offsetof(struct virtio_net_config,
>> > > mtu));
>> > > - if (virtnet_change_mtu(dev, mtu))
>> > > + if (mtu < dev->min_mtu || mtu > dev->max_mtu)
>> >
>> > In fact the > max_mtu branch does not make sense since a 16 bit
>> > value can't exceed MAX_MTU.
>>
>> Hm. mtu is declared as an int, not sure if there's any sort of type
>> promotion to be worried about (not an area I know much/anything about).
>
> Not by design, that's for sure.
If you're really worried, we could declare it as a u16. The value
returned from virtio_cread16 is type u16, and there are no type
promotion rules I'm aware of that would do the wrong thing there.
>> Certainly something that could be looked into as a minor optimization,
>> though it's only in a probe path and shouldn't hurt anything, so ... meh?
>
> Right. Aaron said he's working on a patch that essentially does
> dev->max_mtu = mtu after validation, so this part will look
> a bit silly there.
Agreed, but I can do that in my patch if you don't want the extra churn.
-Aaron
^ permalink raw reply
* Re: [PATCH net-next v6 2/3] udp: implement memory accounting helpers
From: Paolo Abeni @ 2016-10-21 13:34 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, David S. Miller, James Morris,
Trond Myklebust, Alexander Duyck, Daniel Borkmann, Eric Dumazet,
Tom Herbert, Hannes Frederic Sowa, Edward Cree,
linux-nfs-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1477052642.7065.63.camel-XN9IlZ5yJG9HTL0Zs8A6p+yfmBU6pStAUsxypvmhUTTZJqsBc5GL+g@public.gmane.org>
On Fri, 2016-10-21 at 05:24 -0700, Eric Dumazet wrote:
> On Fri, 2016-10-21 at 13:55 +0200, Paolo Abeni wrote:
> > Avoid using the generic helpers.
> > Use the receive queue spin lock to protect the memory
> > accounting operation, both on enqueue and on dequeue.
> >
> > On dequeue perform partial memory reclaiming, trying to
> > leave a quantum of forward allocated memory.
> >
> > On enqueue use a custom helper, to allow some optimizations:
> > - use a plain spin_lock() variant instead of the slightly
> > costly spin_lock_irqsave(),
> > - avoid dst_force check, since the calling code has already
> > dropped the skb dst
> > - avoid orphaning the skb, since skb_steal_sock() already did
> > the work for us
>
> >
> > +static void udp_rmem_release(struct sock *sk, int size, int partial)
> > +{
> > + int amt;
> > +
> > + atomic_sub(size, &sk->sk_rmem_alloc);
> > +
> > + spin_lock_bh(&sk->sk_receive_queue.lock);
> > + sk->sk_forward_alloc += size;
> > + amt = (sk->sk_forward_alloc - partial) & ~(SK_MEM_QUANTUM - 1);
> > + sk->sk_forward_alloc -= amt;
> > + spin_unlock_bh(&sk->sk_receive_queue.lock);
> > +
> > + if (amt)
> > + __sk_mem_reduce_allocated(sk, amt >> SK_MEM_QUANTUM_SHIFT);
> > +}
> > +
> > +static void udp_rmem_free(struct sk_buff *skb)
> > +{
> > + udp_rmem_release(skb->sk, skb->truesize, 1);
> > +}
> > +
>
>
> It looks like you are acquiring/releasing sk_receive_queue.lock twice
> per packet in recvmsg() (the second time in the destructor above)
>
> We could do slightly better if :
>
> We do not set skb->destructor at all, and manage
> sk_rmem_alloc/sk_forward_alloc changes at the time we dequeue skb
> (if !MSG_PEEK), before copy to user space.
Thank you again for reviewing this!
Updating sk_rmem_alloc would still need an atomic operation,
because it is touched also by the error queue path: we will end up
adding an atomic operation (or two, when reclaiming the fwd allocated
memory) inside the critical section. The contention will likely
increase.
The above is going to be quite intrusive: we need to pass an
additional argument all the way up to __skb_try_recv_datagram() and
change the function behavior according to its value.
If you are otherwise satisfied with the series in the current shape,
can't we instead build incrementally on top of it ? it will simplify
both reviews and re-factor tasks.
Thank you,
Paolo
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: commit-3ac72b7b63d5 breaks networking on iMX28
From: Eric Nelson @ 2016-10-21 13:39 UTC (permalink / raw)
To: Henri Roosen, fugang.duan, netdev@vger.kernel.org
In-Reply-To: <bc6aa851-3f52-2274-47dc-4933662648a1@ginzinger.com>
Hi Henri,
On 10/21/2016 02:19 AM, Henri Roosen wrote:
> Hi,
>
> Unfortunately commit-3ac72b7b63d5 "net: fec: align IP header in
> hardware" breaks networking on an iMX28 system.
>
> The commit seems valid for iMX6, where it is tested okay and solves the
> unaligned accesses.
>
> On iMX28 I still see unaligned accesses and networking is broken. Can
> anyone confirm this?
>
Reading the i.MX28 reference manual, it appears that this SoC doesn't
have the RACC bit, and I missed removal of the FEC_QUIRK_HAS_RACC flag.
Can you try with this patch?
diff --git a/drivers/net/ethernet/freescale/fec_main.c
b/drivers/net/ethernet/freescale/fec_main.c
index 692ee24..db5eee9 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -96,7 +96,7 @@ static struct platform_device_id fec_devtype[] = {
}, {
.name = "imx28-fec",
.driver_data = FEC_QUIRK_ENET_MAC | FEC_QUIRK_SWAP_FRAME |
- FEC_QUIRK_SINGLE_MDIO | FEC_QUIRK_HAS_RACC,
+ FEC_QUIRK_SINGLE_MDIO,
}, {
.name = "imx6q-fec",
.driver_data = FEC_QUIRK_ENET_MAC | FEC_QUIRK_HAS_GBIT |
Please advise,
Eric
^ permalink raw reply related
* Re: [PATCH net-next v6 2/3] udp: implement memory accounting helpers
From: Eric Dumazet @ 2016-10-21 13:41 UTC (permalink / raw)
To: Paolo Abeni
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, David S. Miller, James Morris,
Trond Myklebust, Alexander Duyck, Daniel Borkmann, Eric Dumazet,
Tom Herbert, Hannes Frederic Sowa, Edward Cree,
linux-nfs-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1477056841.4606.36.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Fri, 2016-10-21 at 15:34 +0200, Paolo Abeni wrote:
> Updating sk_rmem_alloc would still need an atomic operation,
> because it is touched also by the error queue path: we will end up
> adding an atomic operation (or two, when reclaiming the fwd allocated
> memory) inside the critical section. The contention will likely
> increase.
>
> The above is going to be quite intrusive: we need to pass an
> additional argument all the way up to __skb_try_recv_datagram() and
> change the function behavior according to its value.
>
> If you are otherwise satisfied with the series in the current shape,
> can't we instead build incrementally on top of it ? it will simplify
> both reviews and re-factor tasks.
I certainly can provide an incremental patch, it might be easier for
both of us ;)
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: commit-3ac72b7b63d5 breaks networking on iMX28
From: Fabio Estevam @ 2016-10-21 13:43 UTC (permalink / raw)
To: Eric Nelson; +Cc: Henri Roosen, Fugang Duan, netdev@vger.kernel.org
In-Reply-To: <fea86fa6-d74a-43ff-cf60-0d375336a5ba@nelint.com>
Hi Eric,
On Fri, Oct 21, 2016 at 11:39 AM, Eric Nelson <eric@nelint.com> wrote:
> Reading the i.MX28 reference manual, it appears that this SoC doesn't
> have the RACC bit, and I missed removal of the FEC_QUIRK_HAS_RACC flag.
On MX28 the register is called HW_ENET_MAC_IPACCRXCONF.
I sent a patch that fixes the issue.
^ permalink raw reply
* Re: commit-3ac72b7b63d5 breaks networking on iMX28
From: Eric Nelson @ 2016-10-21 13:46 UTC (permalink / raw)
To: Henri Roosen, fugang.duan, netdev@vger.kernel.org
In-Reply-To: <fea86fa6-d74a-43ff-cf60-0d375336a5ba@nelint.com>
Hi Henri,
On 10/21/2016 06:39 AM, Eric Nelson wrote:
> Hi Henri,
>
> On 10/21/2016 02:19 AM, Henri Roosen wrote:
>> Hi,
>>
>> Unfortunately commit-3ac72b7b63d5 "net: fec: align IP header in
>> hardware" breaks networking on an iMX28 system.
>>
>> The commit seems valid for iMX6, where it is tested okay and solves the
>> unaligned accesses.
>>
>> On iMX28 I still see unaligned accesses and networking is broken. Can
>> anyone confirm this?
>>
>
> Reading the i.MX28 reference manual, it appears that this SoC doesn't
> have the RACC bit, and I missed removal of the FEC_QUIRK_HAS_RACC flag.
>
Sorry. I was wrong.
The register (offset 0x1c4) is present, including the SHIFT16 bit, but
the register is named differently (HW_ENET_MAC_IPACCRXCONF).
^ permalink raw reply
* Re: commit-3ac72b7b63d5 breaks networking on iMX28
From: Eric Nelson @ 2016-10-21 13:48 UTC (permalink / raw)
To: Fabio Estevam; +Cc: Henri Roosen, Fugang Duan, netdev@vger.kernel.org
In-Reply-To: <CAOMZO5DG6=pNDYgT4Bf4KBj4UPOwemBHNk6PYd8DZKt6Yp0PfQ@mail.gmail.com>
Thanks Fabio.
On 10/21/2016 06:43 AM, Fabio Estevam wrote:
> Hi Eric,
>
> On Fri, Oct 21, 2016 at 11:39 AM, Eric Nelson <eric@nelint.com> wrote:
>
>> Reading the i.MX28 reference manual, it appears that this SoC doesn't
>> have the RACC bit, and I missed removal of the FEC_QUIRK_HAS_RACC flag.
>
> On MX28 the register is called HW_ENET_MAC_IPACCRXCONF.
>
> I sent a patch that fixes the issue.
>
I saw that. Nice catch!
^ 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