* Re: [net-next PATCH 6/8] mlx4: Add support for inner IPv6 checksum offloads and TSO
From: Tariq Toukan @ 2016-04-28 13:26 UTC (permalink / raw)
To: Alexander Duyck, Saeed Mahameed
Cc: Alex Duyck, Tal Alon, Linux Kernel Network Developers,
David Miller, Gal Pressman, Or Gerlitz, Eran Ben Elisha
In-Reply-To: <5720FF84.9020404@gmail.com>
On 27/04/2016 9:05 PM, Alexander Duyck wrote:
> On 04/27/2016 08:39 AM, Tariq Toukan wrote:
>>
>>
>> On 27/04/2016 12:01 AM, Alexander Duyck wrote:
>>> On Tue, Apr 26, 2016 at 1:23 PM, Saeed Mahameed
>>> <saeedm@dev.mellanox.co.il> wrote:
>>>> On Tue, Apr 26, 2016 at 6:50 PM, Alex Duyck <aduyck@mirantis.com>
>>>> wrote:
>>>>> The setup is pretty straight forward. Basically I left the first
>>>>> port
>>>>> in the default namespace and moved the second int a secondary
>>>>> namespace referred to below as $netns. I then assigned the IPv6
>>>>> addresses fec0::10:1 and fec0::10:2. After that I ran the following:
>>>>>
>>>>> VXLAN=vx$net
>>>>> echo $VXLAN ${test_options[$i]}
>>>>> ip link add $VXLAN type vxlan id $net \
>>>>> local fec0::10:1 remote $addr6 dev $PF0 \
>>>>> ${test_options[$i]} dstport `expr 8800 + $net`
>>>>> ip netns exec $netns ip link add $VXLAN type vxlan id $net \
>>>>> local $addr6 remote fec0::10:1
>>>>> dev $port \
>>>>> ${test_options[$i]} dstport `expr
>>>>> 8800 + $net`
>>>>> ifconfig $VXLAN 192.168.${net}.1/24
>>>>> ip netns exec $netns ifconfig $VXLAN 192.168.${net}.2/24
>>>>>
>>>> Thanks, indeed i see that GSO is not working with vxlan over IPv6 over
>>>> mlx5 device.
>>>> We will test out those patches on both mlx4 and mlx5, and debug mlx4
>>>> IPv6 issue you see.
>>>>
>>>>>> Anyway, I suspect it might be related to a driver bug most likely in
>>>>>> get_real_size function @en_tx.c
>>>>>> specifically in : *lso_header_size =
>>>>>> (skb_inner_transport_header(skb) -
>>>>>> skb->data) + inner_tcp_hdrlen(skb);
>>>>>>
>>>>>> will check this and get back to you.
>>>>> I'm not entirely convinced. What I was seeing is t hat the hardware
>>>>> itself was performing Rx checksum offload only on tunnels with an
>>>>> outer IPv4 header and ignoring tunnels with an outer IPv6 header.
>>>> I don't get it, are you trying to say that the issue is in the RX
>>>> side ?
>>>> what do you mean by ignoring ? Dropping ? or just not validating
>>>> checksum ?
>>>> if so why would you disable GSO and IPv6 checksumming on TX ?
>>> I'm suspecting that whatever parsing logic exists in either the
>>> hardware or firmware may not be configured to parse tunnels with outer
>>> IPv6 headers. The tell-tale sign is what occurs with an IPv6 based
>>> tunnel with no outer checksum. The hardware is not performing a
>>> checksum on the inner headers so it reports it as a UDP frame with no
>>> checksum to the stack which ends up preventing us from doing GRO.
>>> That tells me that the hardware is not parsing IPv6 based tunnels on
>>> Rx. I am assuming that if the Rx side doesn't work then there is a
>>> good chance that the Tx won't.
>>>
>>>>>>> @@ -2431,7 +2435,18 @@ static netdev_features_t
>>>>>>> mlx4_en_features_check(struct sk_buff *skb,
>>>>>>> netdev_features_t
>>>>>>> features)
>>>>>>> {
>>>>>>> features = vlan_features_check(skb, features);
>>>>>>> - return vxlan_features_check(skb, features);
>>>>>>> + features = vxlan_features_check(skb, features);
>>>>>>> +
>>>>>>> + /* The ConnectX-3 doesn't support outer IPv6 checksums but
>>>>>>> it does
>>>>>>> + * support inner IPv6 checksums and segmentation so we
>>>>>>> need to
>>>>>>> + * strip that feature if this is an IPv6 encapsulated
>>>>>>> frame.
>>>>>>> + */
>>>>>>> + if (skb->encapsulation &&
>>>>>>> + (skb->ip_summed == CHECKSUM_PARTIAL) &&
>>>>>>> + (ip_hdr(skb)->version != 4))
>>>>>>> + features &= ~(NETIF_F_CSUM_MASK |
>>>>>>> NETIF_F_GSO_MASK);
>>>>>> Dejavu, didn't you fix this already in harmonize_features, in
>>>>>> i.e, it is enough to do here:
>>>>>>
>>>>>> if (skb->encapsulation && (skb->ip_summed == CHECKSUM_PARTIAL))
>>>>>> features &= ~NETIF_F_IPV6_CSUM;
>>>>>>
>>>>> So what this patch is doing is enabling an inner IPv6 header
>>>>> offloads.
>>>>> Up above we set the NETIF_F_IPV6_CSUM bit and we want it to stay set
>>>>> unless we have an outer IPv6 header because the inner headers may
>>>>> still need that bit set. If I did what you suggest it strips IPv6
>>>>> checksum support for inner headers and if we have to use GSO
>>>>> partial I
>>>>> ended up encountering some of the other bugs that I have fixed for
>>>>> GSO
>>>>> partial where either sg or csum are not defined.
>>>>>
>>>> I see, you mean that you want to disable checksumming and GSO only for
>>>> packets with Outer(IPv6):Inner(X) and keep it in case for
>>>> Outer(IPv4):Inner(IPv6)
>>>> but i think it is weird that the driver decides to disable features it
>>>> didn't declare in first place (NETIF_F_CSUM_MASK | NETIF_F_GSO_MASK)
>>>>
>>>> Retry:
>>>>
>>>> if (skb->encapsulation && (skb->ip_summed == CHECKSUM_PARTIAL) &&
>>>> (ip_hdr(skb)->version != 4))
>>>> features &= ~NETIF_F_IPV6_CSUM;
>>>>
>>>> will this work ?
>>> Sort of. All that would happen is that you would fall through to
>>> harmonize_features where NETIF_F_CSUM_MASK | NETIF_F_GSO_MASK gets
>>> cleared. I just figured I would short-cut things since we cannot
>>> support inner checksum or any GSO offloads if the tunnel has an outer
>>> IPv6 header. In addition this happens to effectively be the same code
>>> I am using in vxlan_features_check to disable things if we cannot
>>> checksum a protocol so it should help to keep the code size smaller
>>> for the function if the compiler is smart enough to coalesce similar
>>> code.
>>>
>>>> Anyway i prefer to debug the mlx4 issue first before we discuss the
>>>> best approach to disable checksumming & GSO for outer IPv6 in mlx4.
>>> The current code as-is already has it disabled. All I am doing is
>>> enabling IPv6 checksums for inner headers as it seems like it doesn't
>>> work for outer headers.
>>
>> Hi Alex,
>> I will be working on the mlx4 issue next week after the holidays.
>> I will check this offline in-house, without blocking the series.
>>
>> Regards,
>> Tariq
>>
>
> If it helps below are the kind of results I am seeing with the patch
> series applied versus what is currently there. The big problem areas
> are IPv6 over IPv4 tunnels, and IPv4 over IPv6 tunnels without checksums.
>
> The breakdown below is bare-ip-version without tunnel, outer-inner
> with tunnel, or outer-csum-inner if a checksum is present on the outer
> UDP header.
>
> After the series is applied you can see the v6 over v4 issues are
> addressed, and GSO partial has improved the performance for traffic
> over v4 tunnels with outer UDP checksums.
>
> Throughput Throughput Local Local Result
> Units CPU Service Tag
> Util Demand
> %
> mlx4 - Before
> -------------------------------------------------
> 26616.45 10^6bits/s 3.62 0.357 "v4"
> 26101.18 10^6bits/s 6.72 0.675 "v6"
> 22289.41 10^6bits/s 6.49 0.764 "v4-v4"
> N/A - could not connect "v4-v6"
> 12743.91 10^6bits/s 4.25 0.874 "v4-csum-v4"
> N/A - could not connect "v4-csum-v6"
> 0.69 10^6bits/s 0.66 2519.1 "v6-v4"
> 5924.47 10^6bits/s 4.23 1.871 "v6-v6"
> 10369.95 10^6bits/s 4.33 1.096 "v6-csum-v4"
> 10648.51 10^6bits/s 4.10 1.010 "v6-csum-v6"
>
> mlx4 - After
> -------------------------------------------------
> 26585.36 10^6bits/s 3.60 0.355 "v4"
> 26342.86 10^6bits/s 6.67 0.664 "v6"
> 22295.93 10^6bits/s 6.34 0.746 "v4-v4"
> 19977.24 10^6bits/s 6.04 0.793 "v4-v6"
> 22164.71 10^6bits/s 6.46 0.763 "v4-csum-v4"
> 19685.22 10^6bits/s 6.12 0.815 "v4-csum-v6"
> 6126.80 10^6bits/s 4.29 1.835 "v6-v4"
> 5793.53 10^6bits/s 4.24 1.917 "v6-v6"
> 10278.52 10^6bits/s 4.07 1.039 "v6-csum-v4"
> 10526.68 10^6bits/s 4.11 1.024 "v6-csum-v6"
Yeah that can help in comparing our results and getting aligned to
address the issues.
Thanks for sharing.
Regards,
Tariq
^ permalink raw reply
* [PATCH net-next] net: snmp: fix 64bit stats on 32bit arches
From: Eric Dumazet @ 2016-04-28 13:33 UTC (permalink / raw)
To: nicolas.dichtel; +Cc: Eric Dumazet, David S . Miller, netdev
In-Reply-To: <5721FFFC.9010808@6wind.com>
From: Eric Dumazet <edumazet@google.com>
I accidentally replaced BH disabling by preemption disabling
in SNMP_ADD_STATS64() and SNMP_UPD_PO_STATS64() on 32bit builds.
For 64bit stats on 32bit arch, we really need to disable BH,
since the "struct u64_stats_sync syncp" might be manipulated
both from process and BH contexts.
Fixes: 6aef70a851ac ("net: snmp: kill various STATS_USER() helpers")
Reported-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Tested-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/net/snmp.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/net/snmp.h b/include/net/snmp.h
index 6bdd255b2250..c9228ad7ee91 100644
--- a/include/net/snmp.h
+++ b/include/net/snmp.h
@@ -166,9 +166,9 @@ struct linux_xfrm_mib {
#define SNMP_ADD_STATS64(mib, field, addend) \
do { \
- preempt_disable(); \
+ local_bh_disable(); \
__SNMP_ADD_STATS64(mib, field, addend); \
- preempt_enable(); \
+ local_bh_enable(); \
} while (0)
#define __SNMP_INC_STATS64(mib, field) SNMP_ADD_STATS64(mib, field, 1)
@@ -184,9 +184,9 @@ struct linux_xfrm_mib {
} while (0)
#define SNMP_UPD_PO_STATS64(mib, basefield, addend) \
do { \
- preempt_disable(); \
+ local_bh_disable(); \
__SNMP_UPD_PO_STATS64(mib, basefield, addend); \
- preempt_enable(); \
+ local_bh_enable(); \
} while (0)
#else
#define __SNMP_INC_STATS64(mib, field) __SNMP_INC_STATS(mib, field)
^ permalink raw reply related
* Re: [net-next PATCH V4 3/5] samples/bpf: add a README file to get users started
From: Naveen N. Rao @ 2016-04-28 13:37 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: netdev, linux-kbuild, bblanco, borkmann, alexei.starovoitov
In-Reply-To: <20160428122103.16807.41837.stgit@firesoul>
On 2016/04/28 02:21PM, Jesper Dangaard Brouer wrote:
> Getting started with using examples in samples/bpf/ is not
> straightforward. There are several dependencies, and specific
> versions of these dependencies.
>
> Just compiling the example tool is also slightly obscure, e.g. one
> need to call make like:
>
> make samples/bpf/
>
> Do notice the "/" slash after the directory name.
>
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
Acked-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> ---
> samples/bpf/README.rst | 64 ++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 64 insertions(+)
> create mode 100644 samples/bpf/README.rst
>
> diff --git a/samples/bpf/README.rst b/samples/bpf/README.rst
> new file mode 100644
> index 000000000000..993d280184fa
> --- /dev/null
> +++ b/samples/bpf/README.rst
> @@ -0,0 +1,64 @@
> +eBPF sample programs
> +====================
> +
> +This directory contains a mini eBPF library, test stubs, verifier
> +test-suite and examples for using eBPF.
> +
> +Build dependencies
> +==================
> +
> +Compiling requires having installed:
> + * clang >= version 3.4.0
> + * llvm >= version 3.7.1
> +
> +Note that LLVM's tool 'llc' must support target 'bpf', list version
> +and supported targets with command: ``llc --version``
> +
> +Kernel headers
> +--------------
> +
> +There are usually dependencies to header files of the current kernel.
> +To avoid installing devel kernel headers system wide, as a normal
> +user, simply call::
> +
> + make headers_install
> +
> +This will creates a local "usr/include" directory in the git/build top
> +level directory, that the make system automatically pickup first.
> +
> +Compiling
> +=========
> +
> +For building the BPF samples, issue the below command from the kernel
> +top level directory::
> +
> + make samples/bpf/
> +
> +Do notice the "/" slash after the directory name.
> +
> +Manually compiling LLVM with 'bpf' support
> +------------------------------------------
> +
> +Since version 3.7.0, LLVM adds a proper LLVM backend target for the
> +BPF bytecode architecture.
> +
> +By default llvm will build all non-experimental backends including bpf.
> +To generate a smaller llc binary one can use::
> +
> + -DLLVM_TARGETS_TO_BUILD="BPF"
> +
> +Quick sniplet for manually compiling LLVM and clang
> +(build dependencies are cmake and gcc-c++)::
> +
> + $ git clone http://llvm.org/git/llvm.git
> + $ cd llvm/tools
> + $ git clone --depth 1 http://llvm.org/git/clang.git
> + $ cd ..; mkdir build; cd build
> + $ cmake .. -DLLVM_TARGETS_TO_BUILD="BPF;X86"
> + $ make -j $(getconf _NPROCESSORS_ONLN)
> +
> +It is also possible to point make to the newly compiled 'llc' command
> +via redefining LLC on the make command line::
> +
> + make samples/bpf/ LLC=~/git/llvm/build/bin/llc
> +
>
^ permalink raw reply
* Re: [PATCH 3.2 085/115] veth: don’t modify ip_summed; doing so treats packets with bad checksums as good.
From: Ben Greear @ 2016-04-28 13:45 UTC (permalink / raw)
To: Sabrina Dubroca
Cc: Hannes Frederic Sowa, Ben Hutchings, linux-kernel, stable, akpm,
David S. Miller, Vijay Pandurangan, Cong Wang, netdev, Evan Jones,
Nicolas Dichtel, Phil Sutter, Toshiaki Makita, xiyou.wangcong
In-Reply-To: <20160428102953.GA7656@bistromath.localdomain>
On 04/28/2016 03:29 AM, Sabrina Dubroca wrote:
> Hello,
>
> 2016-04-27, 17:14:44 -0700, Ben Greear wrote:
>> On 04/27/2016 05:00 PM, Hannes Frederic Sowa wrote:
>>> Hi Ben,
>>>
>>> On Wed, Apr 27, 2016, at 20:07, Ben Hutchings wrote:
>>>> On Wed, 2016-04-27 at 08:59 -0700, Ben Greear wrote:
>>>>> On 04/26/2016 04:02 PM, Ben Hutchings wrote:
>>>>>>
>>>>>> 3.2.80-rc1 review patch. If anyone has any objections, please let me know.
>>>>> I would be careful about this. It causes regressions when sending
>>>>> PACKET_SOCKET buffers from user-space to veth devices.
>>>>>
>>>>> There was a proposed upstream fix for the regression, but it has not gone
>>>>> into the tree as far as I know.
>>>>>
>>>>> http://www.spinics.net/lists/netdev/msg370436.html
>>>> [...]
>>>>
>>>> OK, I'll drop this for now.
>>>
>>> The fall out from not having this patch is in my opinion a bigger
>>> fallout than not having this patch. This patch fixes silent data
>>> corruption vs. the problem Ben Greear is talking about, which might not
>>> be that a common usage.
>>>
>>> What do others think?
>>>
>>> Bye,
>>> Hannes
>>>
>>
>> This patch from Cong Wang seems to fix the regression for me, I think it should be added and
>> tested in the main tree, and then apply them to stable as a pair.
>>
>> http://dmz2.candelatech.com/?p=linux-4.4.dev.y/.git;a=commitdiff;h=8153e983c0e5eba1aafe1fc296248ed2a553f1ac;hp=454b07405d694dad52e7f41af5816eed0190da8a
>
> Actually, no, this is not really a regression.
>
> If you capture packets on a device with checksum offloading enabled,
> the TCP/UDP checksum isn't filled. veth also behaves that way. What
> the "veth: don't modify ip_summed" patch does is enable proper
> checksum validation on veth. This really was a bug in veth.
>
> Cong's patch would also break cases where we choose to inject packets
> with invalid checksums, and they would now be accepted as correct.
>
> Your use case is invalid, it just happened to work because of a
> bug. If you want the stack to fill checksums so that you want capture
> and reinject packets, you have to disable checksum offloading (or
> compute the checksum yourself in userspace).
Disabling checksum offloading or computing in user-space (and then
recomputing in veth to verify the checksum?) is a huge performance loss.
Maybe we could add a socket option to enable Cong's patch on a per-socket
basis? That way my use-case can still work and you can have this new
behaviour by default?
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* [PATCH net-next] net: ethernet: stmmac: update MDIO support for GMAC4
From: Alexandre TORGUE @ 2016-04-28 13:56 UTC (permalink / raw)
To: netdev, peppe.cavallaro
On new GMAC4 IP, MAC_MDIO_address register has been updated, and bitmaps
changed. This patch takes into account those changes.
Signed-off-by: Alexandre TORGUE <alexandre.torgue@st.com>
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
index 06704ca..3f83c36 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
@@ -37,6 +37,18 @@
#define MII_BUSY 0x00000001
#define MII_WRITE 0x00000002
+/* GMAC4 defines */
+#define MII_GMAC4_GOC_SHIFT 2
+#define MII_GMAC4_WRITE (1 << MII_GMAC4_GOC_SHIFT)
+#define MII_GMAC4_READ (3 << MII_GMAC4_GOC_SHIFT)
+
+#define MII_PHY_ADDR_GMAC4_SHIFT 21
+#define MII_PHY_ADDR_GMAC4_MASK GENMASK(25, 21)
+#define MII_PHY_REG_GMAC4_SHIFT 16
+#define MII_PHY_REG_GMAC4_MASK GENMASK(20, 16)
+#define MII_CSR_CLK_GMAC4_SHIFT 8
+#define MII_CSR_CLK_GMAC4_MASK GENMASK(11, 8)
+
static int stmmac_mdio_busy_wait(void __iomem *ioaddr, unsigned int mii_addr)
{
unsigned long curr;
@@ -124,6 +136,80 @@ static int stmmac_mdio_write(struct mii_bus *bus, int phyaddr, int phyreg,
}
/**
+ * stmmac_mdio_read_gmac4
+ * @bus: points to the mii_bus structure
+ * @phyaddr: MII addr reg bits 25-21
+ * @phyreg: MII addr reg bits 20-16
+ * Description: it reads data from the MII register of GMAC4 from within
+ * the phy device.
+ */
+static int stmmac_mdio_read_gmac4(struct mii_bus *bus, int phyaddr, int phyreg)
+{
+ struct net_device *ndev = bus->priv;
+ struct stmmac_priv *priv = netdev_priv(ndev);
+ unsigned int mii_address = priv->hw->mii.addr;
+ unsigned int mii_data = priv->hw->mii.data;
+ int data;
+ u32 value = (((phyaddr << MII_PHY_ADDR_GMAC4_SHIFT) &
+ (MII_PHY_ADDR_GMAC4_MASK)) |
+ ((phyreg << MII_PHY_REG_GMAC4_SHIFT) &
+ (MII_PHY_REG_GMAC4_MASK))) | MII_GMAC4_READ;
+
+ value |= MII_BUSY | ((priv->clk_csr & MII_CSR_CLK_GMAC4_MASK)
+ << MII_CSR_CLK_GMAC4_SHIFT);
+
+ if (stmmac_mdio_busy_wait(priv->ioaddr, mii_address))
+ return -EBUSY;
+
+ writel(value, priv->ioaddr + mii_address);
+
+ if (stmmac_mdio_busy_wait(priv->ioaddr, mii_address))
+ return -EBUSY;
+
+ /* Read the data from the MII data register */
+ data = (int)readl(priv->ioaddr + mii_data);
+
+ return data;
+}
+
+/**
+ * stmmac_mdio_write_gmac4
+ * @bus: points to the mii_bus structure
+ * @phyaddr: MII addr reg bits 25-21
+ * @phyreg: MII addr reg bits 20-16
+ * @phydata: phy data
+ * Description: it writes the data into the MII register of GMAC4 from within
+ * the device.
+ */
+static int stmmac_mdio_write_gmac4(struct mii_bus *bus, int phyaddr, int phyreg,
+ u16 phydata)
+{
+ struct net_device *ndev = bus->priv;
+ struct stmmac_priv *priv = netdev_priv(ndev);
+ unsigned int mii_address = priv->hw->mii.addr;
+ unsigned int mii_data = priv->hw->mii.data;
+
+ u32 value = (((phyaddr << MII_PHY_ADDR_GMAC4_SHIFT) &
+ (MII_PHY_ADDR_GMAC4_MASK)) |
+ ((phyreg << MII_PHY_REG_GMAC4_SHIFT) &
+ (MII_PHY_REG_GMAC4_MASK))) | MII_GMAC4_WRITE;
+
+ value |= MII_BUSY | ((priv->clk_csr & MII_CSR_CLK_GMAC4_MASK)
+ << MII_CSR_CLK_GMAC4_SHIFT);
+
+ /* Wait until any existing MII operation is complete */
+ if (stmmac_mdio_busy_wait(priv->ioaddr, mii_address))
+ return -EBUSY;
+
+ /* Set the MII address register to write */
+ writel(phydata, priv->ioaddr + mii_data);
+ writel(value, priv->ioaddr + mii_address);
+
+ /* Wait until any existing MII operation is complete */
+ return stmmac_mdio_busy_wait(priv->ioaddr, mii_address);
+}
+
+/**
* stmmac_mdio_reset
* @bus: points to the mii_bus structure
* Description: reset the MII bus
@@ -180,9 +266,11 @@ int stmmac_mdio_reset(struct mii_bus *bus)
/* This is a workaround for problems with the STE101P PHY.
* It doesn't complete its reset until at least one clock cycle
- * on MDC, so perform a dummy mdio read.
+ * on MDC, so perform a dummy mdio read. To be upadted for GMAC4
+ * if needed.
*/
- writel(0, priv->ioaddr + mii_address);
+ if (!priv->plat->has_gmac4)
+ writel(0, priv->ioaddr + mii_address);
#endif
return 0;
}
@@ -217,8 +305,14 @@ int stmmac_mdio_register(struct net_device *ndev)
#endif
new_bus->name = "stmmac";
- new_bus->read = &stmmac_mdio_read;
- new_bus->write = &stmmac_mdio_write;
+ if (priv->plat->has_gmac4) {
+ new_bus->read = &stmmac_mdio_read_gmac4;
+ new_bus->write = &stmmac_mdio_write_gmac4;
+ } else {
+ new_bus->read = &stmmac_mdio_read;
+ new_bus->write = &stmmac_mdio_write;
+ }
+
new_bus->reset = &stmmac_mdio_reset;
snprintf(new_bus->id, MII_BUS_ID_SIZE, "%s-%x",
new_bus->name, priv->plat->bus_id);
--
1.9.1
^ permalink raw reply related
* Re: [PATCH net-next] drivers/net: add 6WIND SHULTI support
From: Or Gerlitz @ 2016-04-28 14:05 UTC (permalink / raw)
To: Jiří Pírko
Cc: David Miller, Florian Westphal, Linux Netdev List,
Nicolas Dichtel
In-Reply-To: <5721FB26.6070305@6wind.com>
On Thu, Apr 28, 2016, Nicolas Dichtel <nicolas.dichtel@6wind.com> wrote:
> Le 27/04/2016 18:55, David Miller a écrit :
>> From: Jiri Pirko <jiri@resnulli.us>
>>> The difference is that it this tries to allow userspace crap to mirror
>>> setting user does for bridge/ovs. Basically this looks to me like an
>>> attempt to enable userspace SDKs and such.
>> +1
> I don't think so because a userspace can receive all the bridge/ovs settings by
> mean of netlink mirror, without the need for this driver at all.
yep, Jiri, can you explain what's the difference in **that** aspect
(sans adding new netlinks) between the case here to the already
existing bridge/bonding (wasn't aware to the ovs ones) reflections to
user-space?
Or.
^ permalink raw reply
* Re: [PATCH net] ip_tunnel: fix preempt warning in ip tunnel creation/updating
From: Paolo Abeni @ 2016-04-28 14:11 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev, David S. Miller, Alexey Kuznetsov, tgraf, pshelar, jbenc,
hannes, sergei.shtylyov, Tom Herbert
In-Reply-To: <1461845394.5535.96.camel@edumazet-glaptop3.roam.corp.google.com>
On Thu, 2016-04-28 at 05:09 -0700, Eric Dumazet wrote:
> On Thu, 2016-04-28 at 11:04 +0200, Paolo Abeni wrote:
> > After the commit e09acddf873b ("ip_tunnel: replace dst_cache with generic
> > implementation"), a preemption debug warning is triggered on ip4
> > tunnels updating; the dst cache helper needs to be invoked in unpreemptible
> > context.
> >
> > We don't need to load the cache on tunnel update, so this commit fixes
> > the warning replacing the load with a dst cache reset, which is
> > preempt safe.
> >
> > Fixes: e09acddf873b ("ip_tunnel: replace dst_cache with generic
> > implementation")
> >
> > Reported-by: Eric Dumazet <eric.dumazet@gmail.com>
> > Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> > ---
>
> Seems good to me, thanks.
>
> Note that the Fixes: tag should not be followed by a blank line.
> (And presumably no line wrap is necessary)
> It really is part of the standard set of tags.
> They should be grouped together.
Oops, I missed that points and checkpatch.pl did not help.
Thank you for the advice.
Paolo
^ permalink raw reply
* [PATCH] net: macb: do not scan PHYs manually
From: Nathan Sullivan @ 2016-04-28 14:19 UTC (permalink / raw)
To: nicolas.ferre; +Cc: netdev, linux-kernel, Nathan Sullivan
Since of_mdiobus_register and mdiobus_register will scan automatically,
do not manually scan for PHY devices in the macb ethernet driver. Doing
so will result in many nonexistent PHYs on the MDIO bus if the MDIO
lines are floating or grounded, such as when they are not used.
Signed-off-by: Nathan Sullivan <nathan.sullivan@ni.com>
---
drivers/net/ethernet/cadence/macb.c | 17 -----------------
1 file changed, 17 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index 48a7d7d..e80e487 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -450,23 +450,6 @@ static int macb_mii_init(struct macb *bp)
if (np) {
/* try dt phy registration */
err = of_mdiobus_register(bp->mii_bus, np);
-
- /* fallback to standard phy registration if no phy were
- found during dt phy registration */
- if (!err && !phy_find_first(bp->mii_bus)) {
- for (i = 0; i < PHY_MAX_ADDR; i++) {
- struct phy_device *phydev;
-
- phydev = mdiobus_scan(bp->mii_bus, i);
- if (IS_ERR(phydev)) {
- err = PTR_ERR(phydev);
- break;
- }
- }
-
- if (err)
- goto err_out_unregister_bus;
- }
} else {
if (pdata)
bp->mii_bus->phy_mask = pdata->phy_mask;
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH net v3 0/5] drivers: net: cpsw: phy-handle fixes
From: Tony Lindgren @ 2016-04-28 14:28 UTC (permalink / raw)
To: David Rivshin (Allworx)
Cc: netdev, linux-omap, Markus Brunner, devicetree, Grygorii Strashko,
Mugunthan V N, Nicolas Chauvet, linux-kernel, Andrew Goodbody,
David Miller, linux-arm-kernel
In-Reply-To: <1461805808-4102-1-git-send-email-drivshin.allworx@gmail.com>
* David Rivshin (Allworx) <drivshin.allworx@gmail.com> [160427 18:13]:
> From: David Rivshin <drivshin@allworx.com>
>
> This series fixes a number of related issues around using phy-handle
> properties in cpsw emac nodes.
>
> Patch 1 fixes a bug if more than one slave is used, and either
> slave uses the phy-handle property in the devicetree.
>
> Patch 2 fixes a NULL pointer dereference which can occur if a
> phy-handle property is used and of_phy_connect() return NULL,
> such as with a bad devicetree.
>
> Patch 3 fixes an issue where the phy-mode property would be ignored
> if a phy-handle property was used. This also fixes a bogus error
> message that would be emitted.
>
> Patch 4 fixes makes the binding documentation more explicit that
> exactly one PHY property should be used, and also marks phy_id as
> deprecated.
>
> Patch 5 cleans up the fixed-link case to work like the now-fixed
> phy-handle case.
>
> I have tested on the following hardware configurations:
> - (EVMSK) dual emac, phy_id property in both slaves
> - (EVMSK) dual emac, phy-handle property in both slaves
> - (EVMSK) a bad phy-handle property pointing to &mmc1
> - (EVMSK) phy_id property with incorrect PHY address
> - (BeagleBoneBlack) single emac, phy_id property
> - (custom) single emac, fixed-link subnode
>
> Andrew Goodbody reported testing v2 on a board that doesn't use
> dual_emac mode, but with 2 PHYs using phy-handle properties [1].
>
> Nicolas Chauvet reported testing v2 on an HP t410 (dm8148).
>
> Markus Brunner reported testing v1 on the following [2]:
> - emac0 with phy_id and emac1 with fixed phy
> - emac0 with phy-handle and emac1 with fixed phy
> - emac0 with fixed phy and emac1 with fixed phy
Quickly boot tested these against next on dra62x-j5eco EVM:
Tested-by: Tony Lindgren <tony@atomide.com>
^ permalink raw reply
* [PATCH net-next] vxlan: fix initialization with custom link parameters
From: Jiri Benc @ 2016-04-28 14:36 UTC (permalink / raw)
To: netdev; +Cc: Nicolas Dichtel
Commit 0c867c9bf84c ("vxlan: move Ethernet initialization to a separate
function") changed initialization order and as an unintended result, when the
user specifies additional link parameters (such as IFLA_ADDRESS) while
creating vxlan interface, those are overwritten by vxlan_ether_setup later.
It's necessary to call ether_setup from withing the ->setup callback. That
way, the correct parameters are set by rtnl_create_link later. This is done
also for VXLAN-GPE, as we don't know the interface type yet at that point,
and changed to the correct interface type later.
Fixes: 0c867c9bf84c ("vxlan: move Ethernet initialization to a separate function")
Reported-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Signed-off-by: Jiri Benc <jbenc@redhat.com>
---
drivers/net/vxlan.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 6fb93b57a724..2668e528dee4 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -2557,6 +2557,9 @@ static void vxlan_setup(struct net_device *dev)
struct vxlan_dev *vxlan = netdev_priv(dev);
unsigned int h;
+ eth_hw_addr_random(dev);
+ ether_setup(dev);
+
dev->destructor = free_netdev;
SET_NETDEV_DEVTYPE(dev, &vxlan_type);
@@ -2592,8 +2595,6 @@ static void vxlan_setup(struct net_device *dev)
static void vxlan_ether_setup(struct net_device *dev)
{
- eth_hw_addr_random(dev);
- ether_setup(dev);
dev->priv_flags &= ~IFF_TX_SKB_SHARING;
dev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
dev->netdev_ops = &vxlan_netdev_ether_ops;
@@ -2601,11 +2602,10 @@ static void vxlan_ether_setup(struct net_device *dev)
static void vxlan_raw_setup(struct net_device *dev)
{
+ dev->header_ops = NULL;
dev->type = ARPHRD_NONE;
dev->hard_header_len = 0;
dev->addr_len = 0;
- dev->mtu = ETH_DATA_LEN;
- dev->tx_queue_len = 1000;
dev->flags = IFF_POINTOPOINT | IFF_NOARP | IFF_MULTICAST;
dev->netdev_ops = &vxlan_netdev_raw_ops;
}
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH] net: macb: do not scan PHYs manually
From: kbuild test robot @ 2016-04-28 14:37 UTC (permalink / raw)
To: Nathan Sullivan
Cc: kbuild-all, nicolas.ferre, netdev, linux-kernel, Nathan Sullivan
In-Reply-To: <1461853187-31715-1-git-send-email-nathan.sullivan@ni.com>
[-- Attachment #1: Type: text/plain, Size: 3784 bytes --]
Hi,
[auto build test WARNING on v4.6-rc5]
[cannot apply to net-next/master next-20160428]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
url: https://github.com/0day-ci/linux/commits/Nathan-Sullivan/net-macb-do-not-scan-PHYs-manually/20160428-222826
config: powerpc-allyesconfig (attached as .config)
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=powerpc
All warnings (new ones prefixed by >>):
drivers/net/ethernet/cadence/macb.c: In function 'macb_mii_init':
>> drivers/net/ethernet/cadence/macb.c:427:20: warning: unused variable 'i' [-Wunused-variable]
int err = -ENXIO, i;
^
vim +/i +427 drivers/net/ethernet/cadence/macb.c
222ca8e0 drivers/net/ethernet/cadence/macb.c Nathan Sullivan 2015-05-22 411 phydev->supported &= ~SUPPORTED_1000baseT_Half;
222ca8e0 drivers/net/ethernet/cadence/macb.c Nathan Sullivan 2015-05-22 412
6c36a707 drivers/net/macb.c frederic RODO 2007-07-12 413 phydev->advertising = phydev->supported;
6c36a707 drivers/net/macb.c frederic RODO 2007-07-12 414
6c36a707 drivers/net/macb.c frederic RODO 2007-07-12 415 bp->link = 0;
6c36a707 drivers/net/macb.c frederic RODO 2007-07-12 416 bp->speed = 0;
6c36a707 drivers/net/macb.c frederic RODO 2007-07-12 417 bp->duplex = -1;
6c36a707 drivers/net/macb.c frederic RODO 2007-07-12 418 bp->phy_dev = phydev;
6c36a707 drivers/net/macb.c frederic RODO 2007-07-12 419
6c36a707 drivers/net/macb.c frederic RODO 2007-07-12 420 return 0;
6c36a707 drivers/net/macb.c frederic RODO 2007-07-12 421 }
6c36a707 drivers/net/macb.c frederic RODO 2007-07-12 422
421d9df0 drivers/net/ethernet/cadence/macb.c Cyrille Pitchen 2015-03-07 423 static int macb_mii_init(struct macb *bp)
6c36a707 drivers/net/macb.c frederic RODO 2007-07-12 424 {
84e0cdb0 drivers/net/ethernet/cadence/macb.c Jamie Iles 2011-03-08 425 struct macb_platform_data *pdata;
148cbb53 drivers/net/ethernet/cadence/macb.c Boris BREZILLON 2013-08-22 426 struct device_node *np;
6c36a707 drivers/net/macb.c frederic RODO 2007-07-12 @427 int err = -ENXIO, i;
6c36a707 drivers/net/macb.c frederic RODO 2007-07-12 428
3dbda77e drivers/net/macb.c Uwe Kleine-Koenig 2009-07-23 429 /* Enable management port */
6c36a707 drivers/net/macb.c frederic RODO 2007-07-12 430 macb_writel(bp, NCR, MACB_BIT(MPE));
6c36a707 drivers/net/macb.c frederic RODO 2007-07-12 431
298cf9be drivers/net/macb.c Lennert Buytenhek 2008-10-08 432 bp->mii_bus = mdiobus_alloc();
298cf9be drivers/net/macb.c Lennert Buytenhek 2008-10-08 433 if (bp->mii_bus == NULL) {
298cf9be drivers/net/macb.c Lennert Buytenhek 2008-10-08 434 err = -ENOMEM;
298cf9be drivers/net/macb.c Lennert Buytenhek 2008-10-08 435 goto err_out;
:::::: The code at line 427 was first introduced by commit
:::::: 6c36a7074436e181fb3df41f66bbdaf53980951e macb: Use generic PHY layer
:::::: TO: frederic RODO <f.rodo@til-technologies.fr>
:::::: CC: Jeff Garzik <jeff@garzik.org>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 48660 bytes --]
^ permalink raw reply
* Re: [net-next PATCH V4 1/5] samples/bpf: add back functionality to redefine LLC command
From: Jesper Dangaard Brouer @ 2016-04-28 14:40 UTC (permalink / raw)
To: Naveen N. Rao
Cc: netdev, linux-kbuild, bblanco, borkmann, alexei.starovoitov,
brouer
In-Reply-To: <20160428132132.GB7880@naverao1-tp.in.ibm.com>
On Thu, 28 Apr 2016 18:51:33 +0530
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
> > Add this features back. Note that it is possible to redefine the LLC
> > on the make command like:
> >
> > make samples/bpf/ LLC=~/git/llvm/build/bin/llc
>
> I don't have an objection to this patch, but you didn't explain why/how
> this approach is better than just doing:
> PATH=~/git/llvm/build/bin make samples/bpf/
It is almost the same. There is always another way to do the same.
I explicitly use this to test different combinations of LLC and CLANG,
in-order to validate Alexei's claim that older versions of CLANG could
still work with a newer version of LLC. Thus, one use-case you
approach cannot cover ;-)
And clang seems to install a clang-3.9, which my solution also covers
by explicitly specifying CLANG=clang-3.9, if several avail clang's are
in the PATH.
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
Author of http://www.iptv-analyzer.org
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [RFC PATCH V2 2/2] vhost: device IOTLB API
From: Michael S. Tsirkin @ 2016-04-28 14:43 UTC (permalink / raw)
To: Jason Wang
Cc: kvm, qemu-devel, netdev, linux-kernel, peterx, virtualization,
pbonzini
In-Reply-To: <5721AF9C.9030209@redhat.com>
On Thu, Apr 28, 2016 at 02:37:16PM +0800, Jason Wang wrote:
>
>
> On 04/27/2016 07:45 PM, Michael S. Tsirkin wrote:
> > On Fri, Mar 25, 2016 at 10:34:34AM +0800, Jason Wang wrote:
> >> This patch tries to implement an device IOTLB for vhost. This could be
> >> used with for co-operation with userspace(qemu) implementation of DMA
> >> remapping.
> >>
> >> The idea is simple. When vhost meets an IOTLB miss, it will request
> >> the assistance of userspace to do the translation, this is done
> >> through:
> >>
> >> - Fill the translation request in a preset userspace address (This
> >> address is set through ioctl VHOST_SET_IOTLB_REQUEST_ENTRY).
> >> - Notify userspace through eventfd (This eventfd was set through ioctl
> >> VHOST_SET_IOTLB_FD).
> > Why use an eventfd for this?
>
> The aim is to implement the API all through ioctls.
>
> > We use them for interrupts because
> > that happens to be what kvm wants, but here - why don't we
> > just add a generic support for reading out events
> > on the vhost fd itself?
>
> I've considered this approach, but what's the advantages of this? I mean
> looks like all other ioctls could be done through vhost fd
> reading/writing too.
read/write have a non-blocking flag.
It's not useful for other ioctls but it's useful here.
> >
> >> - device IOTLB were started and stopped through VHOST_RUN_IOTLB ioctl
> >>
> >> When userspace finishes the translation, it will update the vhost
> >> IOTLB through VHOST_UPDATE_IOTLB ioctl. Userspace is also in charge of
> >> snooping the IOTLB invalidation of IOMMU IOTLB and use
> >> VHOST_UPDATE_IOTLB to invalidate the possible entry in vhost.
> > There's one problem here, and that is that VQs still do not undergo
> > translation. In theory VQ could be mapped in such a way
> > that it's not contigious in userspace memory.
>
> I'm not sure I get the issue, current vhost API support setting
> desc_user_addr, used_user_addr and avail_user_addr independently. So
> looks ok? If not, looks not a problem to device IOTLB API itself.
The problem is that addresses are all HVA.
Without an iommu, we ask for them to be contigious and
since bus address == GPA, this means contigious GPA =>
contigious HVA. With an IOMMU you can map contigious
bus address but non contigious GPA and non contigious HVA.
Another concern: what if guest changes the GPA while keeping bus address
constant? Normal devices will work because they only use
bus addresses, but virtio will break.
> >
> >
> >> Signed-off-by: Jason Wang <jasowang@redhat.com>
> > What limits amount of entries that kernel keeps around?
>
> It depends on guest working set I think. Looking at
> http://dpdk.org/doc/guides/linux_gsg/sys_reqs.html:
>
> - For 2MB page size in guest, it suggests hugepages=1024
> - For 1GB page size, it suggests a hugepages=4
>
> So I choose 2048 to make sure it can cover this.
4K page size is rather common, too.
> > Do we want at least a mod parameter for this?
>
> Maybe.
>
> >
> >> ---
> >> drivers/vhost/net.c | 6 +-
> >> drivers/vhost/vhost.c | 301 +++++++++++++++++++++++++++++++++++++++------
> >> drivers/vhost/vhost.h | 17 ++-
> >> fs/eventfd.c | 3 +-
> >> include/uapi/linux/vhost.h | 35 ++++++
> >> 5 files changed, 320 insertions(+), 42 deletions(-)
> >>
>
> [...]
>
> >> +struct vhost_iotlb_entry {
> >> + __u64 iova;
> >> + __u64 size;
> >> + __u64 userspace_addr;
> > Alignment requirements?
>
> The API does not require any alignment. Will add a comment for this.
>
> >
> >> + struct {
> >> +#define VHOST_ACCESS_RO 0x1
> >> +#define VHOST_ACCESS_WO 0x2
> >> +#define VHOST_ACCESS_RW 0x3
> >> + __u8 perm;
> >> +#define VHOST_IOTLB_MISS 1
> >> +#define VHOST_IOTLB_UPDATE 2
> >> +#define VHOST_IOTLB_INVALIDATE 3
> >> + __u8 type;
> >> +#define VHOST_IOTLB_INVALID 0x1
> >> +#define VHOST_IOTLB_VALID 0x2
> >> + __u8 valid;
> > why do we need this flag?
>
> Useless, will remove.
>
> >
> >> + __u8 u8_padding;
> >> + __u32 padding;
> >> + } flags;
> >> +};
> >> +
> >> +struct vhost_vring_iotlb_entry {
> >> + unsigned int index;
> >> + __u64 userspace_addr;
> >> +};
> >> +
> >> struct vhost_memory_region {
> >> __u64 guest_phys_addr;
> >> __u64 memory_size; /* bytes */
> >> @@ -127,6 +153,15 @@ struct vhost_memory {
> >> /* Set eventfd to signal an error */
> >> #define VHOST_SET_VRING_ERR _IOW(VHOST_VIRTIO, 0x22, struct vhost_vring_file)
> >>
> >> +/* IOTLB */
> >> +/* Specify an eventfd file descriptor to signle on IOTLB miss */
> > typo
>
> Will fix it.
>
> >
> >> +#define VHOST_SET_VRING_IOTLB_CALL _IOW(VHOST_VIRTIO, 0x23, struct \
> >> + vhost_vring_file)
> >> +#define VHOST_SET_VRING_IOTLB_REQUEST _IOW(VHOST_VIRTIO, 0x25, struct \
> >> + vhost_vring_iotlb_entry)
> >> +#define VHOST_UPDATE_IOTLB _IOW(VHOST_VIRTIO, 0x24, struct vhost_iotlb_entry)
> >> +#define VHOST_RUN_IOTLB _IOW(VHOST_VIRTIO, 0x26, int)
> >> +
> > Is the assumption that userspace must dedicate a thread to running the iotlb?
> > I dislike this one.
> > Please support asynchronous APIs at least optionally to make
> > userspace make its own threading decisions.
>
> Nope, my qemu patches does not use a dedicated thread. This API is used
> to start or top DMAR according to e.g whether guest enable DMAR for
> intel IOMMU.
I see. Seems rather confusing - do we need to start/stop it
while device is running?
> >
> >> /* VHOST_NET specific defines */
> >>
> >> /* Attach virtio net ring to a raw socket, or tap device.
> > Don't we need a feature bit for this?
>
> Yes we need it. The feature bit is not considered in this patch and
> looks like it was still under discussion. After we finalize it, I will add.
>
> > Are we short on feature bits? If yes maybe it's time to add
> > something like PROTOCOL_FEATURES that we have in vhost-user.
> >
>
> I believe it can just work like VERSION_1, or is there anything I missed?
VERSION_1 is a virtio feature though. This one would be backend specific
...
--
MST
^ permalink raw reply
* [PATCH v2] net: macb: do not scan PHYs manually
From: Nathan Sullivan @ 2016-04-28 14:46 UTC (permalink / raw)
To: nicolas.ferre; +Cc: netdev, linux-kernel, Nathan Sullivan
Since of_mdiobus_register and mdiobus_register will scan automatically,
do not manually scan for PHY devices in the macb ethernet driver. Doing
so will result in many nonexistent PHYs on the MDIO bus if the MDIO
lines are floating or grounded, such as when they are not used.
Signed-off-by: Nathan Sullivan <nathan.sullivan@ni.com>
---
drivers/net/ethernet/cadence/macb.c | 19 +------------------
1 file changed, 1 insertion(+), 18 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index 48a7d7d..6506b4e 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -424,7 +424,7 @@ static int macb_mii_init(struct macb *bp)
{
struct macb_platform_data *pdata;
struct device_node *np;
- int err = -ENXIO, i;
+ int err = -ENXIO;
/* Enable management port */
macb_writel(bp, NCR, MACB_BIT(MPE));
@@ -450,23 +450,6 @@ static int macb_mii_init(struct macb *bp)
if (np) {
/* try dt phy registration */
err = of_mdiobus_register(bp->mii_bus, np);
-
- /* fallback to standard phy registration if no phy were
- found during dt phy registration */
- if (!err && !phy_find_first(bp->mii_bus)) {
- for (i = 0; i < PHY_MAX_ADDR; i++) {
- struct phy_device *phydev;
-
- phydev = mdiobus_scan(bp->mii_bus, i);
- if (IS_ERR(phydev)) {
- err = PTR_ERR(phydev);
- break;
- }
- }
-
- if (err)
- goto err_out_unregister_bus;
- }
} else {
if (pdata)
bp->mii_bus->phy_mask = pdata->phy_mask;
--
1.7.10.4
^ permalink raw reply related
* Re: [net-next PATCH V4 1/5] samples/bpf: add back functionality to redefine LLC command
From: Naveen N. Rao @ 2016-04-28 14:50 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: netdev, linux-kbuild, bblanco, borkmann, alexei.starovoitov
In-Reply-To: <20160428164046.0998f03d@redhat.com>
On 2016/04/28 04:40PM, Jesper Dangaard Brouer wrote:
> On Thu, 28 Apr 2016 18:51:33 +0530
> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
>
> > > Add this features back. Note that it is possible to redefine the LLC
> > > on the make command like:
> > >
> > > make samples/bpf/ LLC=~/git/llvm/build/bin/llc
> >
> > I don't have an objection to this patch, but you didn't explain why/how
> > this approach is better than just doing:
> > PATH=~/git/llvm/build/bin make samples/bpf/
>
> It is almost the same. There is always another way to do the same.
>
> I explicitly use this to test different combinations of LLC and CLANG,
> in-order to validate Alexei's claim that older versions of CLANG could
> still work with a newer version of LLC. Thus, one use-case you
> approach cannot cover ;-)
>
> And clang seems to install a clang-3.9, which my solution also covers
> by explicitly specifying CLANG=clang-3.9, if several avail clang's are
> in the PATH.
Ah, so a very niche use-case.
Acked-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
- Naveen
^ permalink raw reply
* Re: [PATCH] ip: add udp_csum, udp6_csum_tx, udp6_csum_rx control flags to ip l2tp add tunnel
From: James Chapman @ 2016-04-28 14:50 UTC (permalink / raw)
To: Wang Shanker; +Cc: netdev
In-Reply-To: <AFE6B2C1-93B5-4782-870D-2C6C81BF6F52@gmail.com>
Yes, that looks like the problem.
The comments in l2tp.h which indicate that the csum attributes are u8
values are wrong. Code in net/l2tp/l2tp_netlink.c accesses these
attributes using nla_get_flag().
Please submit a patch to fix l2tp_tunnel_sock_create(). Include good
change notes and your signed-off-by tag so that it gets reviewed. See
Documentation/SubmittingPatches if you haven't submitted a kernel
patch here before.
On 27 April 2016 at 18:06, Wang Shanker <shankerwangmiao@gmail.com> wrote:
>
>
>> 在 2016年4月27日，23:33，Wang Shanker <shankerwangmiao@gmail.com> 写道：
>>
>>
>>
>>> 在 2016年4月27日，20:21，James Chapman <jchapman@katalix.com> 写道：
>>>
>>> On 26 April 2016 at 15:15, Wang Shanker <shankerwangmiao@gmail.com> wrote:
>>>> Hi, all
>>>>
>>>> It’s my first time to contribute to such an important open source project. Things began when I upgraded my server, called "Server A", form ubuntu 14.04 to 16.04, which is shipped with new kernel version, 4.4. After upgrade, I soon found a l2tp tunnel between this server and another linux server, called "Server B", via ipv6 broke down. Here is the network topology:
>>>>
>>>> +----------+ +----------+
>>>> | Server A | -- IPV6 Network -- | Server B |
>>>> +----------+ +----------+
>>>>
>>>> The l2tp tunnel was encapsulated in udp datagrams. All the configuration was normal and could work after I reverted the kernel on Server A to original version.
>>>>
>>>> Here is what i did to create the tunnel:
>>>>
>>>> ```
>>>> on Server A:
>>>>
>>>> ip l2tp add tunnel tunnel_id 86 peer_tunnel_id 86 remote 2001:db8::aaaa local 2001:db8::bbbb udp_sport 1086 udp_dport 1086
>>>> ip l2tp add session name l2tpeth0 tunnel_id 86 session_id 86 peer_session_id 86
>>>> ip l s l2tpeth0 up
>>>>
>>>> on Server B:
>>>>
>>>> ip l2tp add tunnel tunnel_id 86 peer_tunnel_id 86 local 2001:db8::aaaa remote 2001:db8::bbbb udp_sport 1086 udp_dport 1086
>>>> ip l2tp add session name l2tpeth0 tunnel_id 86 session_id 86 peer_session_id 86
>>>> ip l s l2tpeth0 up
>>>>
>>>> ```
>>>>
>>>> When I used tcpdump to diagnose the problem, I got such result:
>>>>
>>>> ```
>>>> on Server A:
>>>>
>>>> arping -i l2tpeth0 -0 1.2.3.4
>>>>
>>>> on Server B:
>>>>
>>>> tcpdump -i eth0 -n port 1086 -v
>>>>
>>>> 21:35:57.818810 IP6 (flowlabel 0x8f028, hlim 64, next-header UDP (17) payload length: 62) 2001:db8::aaaa.1086 > 2001:db8::bbbb.1086: [bad udp cksum 0x0000 -> 0x1140!] UDP, length 54
>>>> 21:35:58.820572 IP6 (flowlabel 0x8f028, hlim 64, next-header UDP (17) payload length: 62) 2001:db8::aaaa.1086 > 2001:db8::bbbb.1086: [bad udp cksum 0x0000 -> 0x1140!] UDP, length 54
>>>> 21:35:59.822216 IP6 (flowlabel 0x8f028, hlim 64, next-header UDP (17) payload length: 62) 2001:db8::aaaa.1086 > 2001:db8::bbbb.1086: [bad udp cksum 0x0000 -> 0x1140!] UDP, length 54
>>>>
>>>> ```
>>>>
>>>> After looking into kernel source, I found out that in this commit a new feature to set udp6 checksum to zero in commit 6b649fe, which added `L2TP_ATTR_UDP_ZERO_CSUM6_TX` and `L2TP_ATTR_UDP_ZERO_CSUM6_TX`.
>>>>
>>>> As a result, I added `udp_csum`, `udp6_csum_tx`, `udp6_csum_rx` control flags to `ip l2tp add tunnel` to control those attributes about checksum.
>>>>
>>>> Using this to create the tunnel instead on Server A:
>>>>
>>>> ```
>>>> ip l2tp add tunnel tunnel_id 86 peer_tunnel_id 86 remote 2001:db8::aaaa local 2001:db8::bbbb udp_sport 1086 udp_dport 1086 udp6_csum_tx on udp6_csum_rx on
>>>> ```
>>>>
>>>> I finally got:
>>>>
>>>> ```
>>>> on Server A:
>>>>
>>>> arping -i l2tpeth0 -0 1.2.3.4
>>>>
>>>> on Server B:
>>>>
>>>> tcpdump -i eth0 -n port 1086 -v
>>>>
>>>> 22:07:03.844297 IP6 (flowlabel 0x8f028, hlim 64, next-header UDP (17) payload length: 62) 2001:db8::aaaa.1086 > 2001:db8::bbbb.1086: [udp sum ok] UDP, length 54
>>>> 22:07:04.845717 IP6 (flowlabel 0x8f028, hlim 64, next-header UDP (17) payload length: 62) 2001:db8::aaaa.1086 > 2001:db8::bbbb.1086: [udp sum ok] UDP, length 54
>>>> 22:07:05.847965 IP6 (flowlabel 0x8f028, hlim 64, next-header UDP (17) payload length: 62) 2001:db8::aaaa.1086 > 2001:db8::bbbb.1086: [udp sum ok] UDP, length 54
>>>>
>>>> tcpdump -i l2tpeth0 -v
>>>>
>>>> 22:10:35.691326 ARP, Ethernet (len 6), IPv4 (len 4), Request who-has 1.2.3.4 tell 0.0.0.0, length 28
>>>> 22:10:36.693627 ARP, Ethernet (len 6), IPv4 (len 4), Request who-has 1.2.3.4 tell 0.0.0.0, length 28
>>>> 22:10:37.695010 ARP, Ethernet (len 6), IPv4 (len 4), Request who-has 1.2.3.4 tell 0.0.0.0, length 28
>>>> 22:10:38.697121 ARP, Ethernet (len 6), IPv4 (len 4), Request who-has 1.2.3.4 tell 0.0.0.0, length 28
>>>>
>>>> ```
>>>>
>>>> It seems to work. However, is it the real point that should be fixed and does my patch fix it well? I need your suggestion.
>>>
>>> This seems reasonable to me. It is good to provide user control of
>>> L2TP checksum options.
>>>
>>> However, there is a problem with your patch. The netlink attributes
>>> you are accessing to control checksums are flags, not u8 values.
>>
>> I’m not so familiar with kernel code. However, in `<linux/l2tp.h>` :
>>
>> ```
>>
>> /*
>> * ATTR types defined for L2TP
>> */
>> enum {
>> L2TP_ATTR_NONE, /* no data */
>> // ...
>> L2TP_ATTR_IP6_DADDR, /* struct in6_addr */
>> L2TP_ATTR_UDP_ZERO_CSUM6_TX, /* u8 */
>> L2TP_ATTR_UDP_ZERO_CSUM6_RX, /* u8 */
>> // ...
>>
>> }
>> ```
>>
>> isn’t L2TP_ATTR_UDP_ZERO_CSUM6_TX a u8 value? Or should I use `addattr` instead of `addattr8`?
>>
>>>
>>> Maybe the default checksum setting for such l2tp tunnels should be
>>> changed in the l2tp kernel code to match the previous behaviour where
>>> IPv6 checksums were disabled?
>>
>> I think so. However, I’m confused with those code.
>>
>> From the name of the attrs `L2TP_ATTR_UDP_ZERO_CSUM6_TX` and `L2TP_ATTR_UDP_ZERO_CSUM6_RX`, I
>> can tell that when those flags are set, the checksum will be zero. Also, according to the
>> comment of commit 6b649fe in kernel source, “Added new L2TP configuration options to allow
>> TX and RX of zero checksums in IPv6. Default is not to use them.”, checksums shouldn't have
>> been zero by default. However, in fact, they are. I think there may be some bugs in kernel
>> source.
>
> I think I’ve got the bug. Here is the patch for kernel
>
> ---
> net/l2tp/l2tp_core.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
> index afca2eb..6edfa99 100644
> --- a/net/l2tp/l2tp_core.c
> +++ b/net/l2tp/l2tp_core.c
> @@ -1376,9 +1376,9 @@ static int l2tp_tunnel_sock_create(struct net *net,
> memcpy(&udp_conf.peer_ip6, cfg->peer_ip6,
> sizeof(udp_conf.peer_ip6));
> udp_conf.use_udp6_tx_checksums =
> - cfg->udp6_zero_tx_checksums;
> + ! cfg->udp6_zero_tx_checksums;
> udp_conf.use_udp6_rx_checksums =
> - cfg->udp6_zero_rx_checksums;
> + ! cfg->udp6_zero_rx_checksums;
> } else
> #endif
> {
> --
>
>
>>>
>>>>
>>>>
>>>> ---
>>>> ip/ipl2tp.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
>>>> 1 file changed, 45 insertions(+)
>>>>
>>>> diff --git a/ip/ipl2tp.c b/ip/ipl2tp.c
>>>> index 3c8ee93..67a6482 100644
>>>> --- a/ip/ipl2tp.c
>>>> +++ b/ip/ipl2tp.c
>>>> @@ -56,6 +56,8 @@ struct l2tp_parm {
>>>>
>>>> uint16_t pw_type;
>>>> uint16_t mtu;
>>>> + int udp6_csum_tx:1;
>>>> + int udp6_csum_rx:1;
>>>> int udp_csum:1;
>>>> int recv_seq:1;
>>>> int send_seq:1;
>>>> @@ -117,6 +119,9 @@ static int create_tunnel(struct l2tp_parm *p)
>>>> if (p->encap == L2TP_ENCAPTYPE_UDP) {
>>>> addattr16(&req.n, 1024, L2TP_ATTR_UDP_SPORT, p->local_udp_port);
>>>> addattr16(&req.n, 1024, L2TP_ATTR_UDP_DPORT, p->peer_udp_port);
>>>> + addattr8 (&req.n, 1024, L2TP_ATTR_UDP_CSUM, p->udp_csum);
>>>> + addattr8 (&req.n, 1024, L2TP_ATTR_UDP_ZERO_CSUM6_TX, p->udp6_csum_tx);
>>>> + addattr8 (&req.n, 1024, L2TP_ATTR_UDP_ZERO_CSUM6_RX, p->udp6_csum_rx);
>>>> }
>>>>
>>>> if (rtnl_talk(&genl_rth, &req.n, NULL, 0) < 0)
>>>> @@ -282,6 +287,14 @@ static int get_response(struct nlmsghdr *n, void *arg)
>>>> p->l2spec_len = rta_getattr_u8(attrs[L2TP_ATTR_L2SPEC_LEN]);
>>>>
>>>> p->udp_csum = !!attrs[L2TP_ATTR_UDP_CSUM];
>>>> + if (attrs[L2TP_ATTR_UDP_ZERO_CSUM6_TX])
>>>> + p->udp6_csum_tx = rta_getattr_u8(attrs[L2TP_ATTR_UDP_ZERO_CSUM6_TX]);
>>>> + else
>>>> + p->udp6_csum_tx = 1;
>>>> + if (attrs[L2TP_ATTR_UDP_ZERO_CSUM6_RX])
>>>> + p->udp6_csum_rx = rta_getattr_u8(attrs[L2TP_ATTR_UDP_ZERO_CSUM6_RX]);
>>>> + else
>>>> + p->udp6_csum_rx = 1;
>>>> if (attrs[L2TP_ATTR_COOKIE])
>>>> memcpy(p->cookie, RTA_DATA(attrs[L2TP_ATTR_COOKIE]),
>>>> p->cookie_len = RTA_PAYLOAD(attrs[L2TP_ATTR_COOKIE]));
>>>> @@ -470,6 +483,9 @@ static void usage(void)
>>>> fprintf(stderr, " tunnel_id ID peer_tunnel_id ID\n");
>>>> fprintf(stderr, " [ encap { ip | udp } ]\n");
>>>> fprintf(stderr, " [ udp_sport PORT ] [ udp_dport PORT ]\n");
>>>> + fprintf(stderr, " [ udp_csum { on | off } ]\n");
>>>> + fprintf(stderr, " [ udp6_csum_tx { on | off } ]\n");
>>>> + fprintf(stderr, " [ udp6_csum_rx { on | off } ]\n");
>>>> fprintf(stderr, "Usage: ip l2tp add session [ name NAME ]\n");
>>>> fprintf(stderr, " tunnel_id ID\n");
>>>> fprintf(stderr, " session_id ID peer_session_id ID\n");
>>>> @@ -500,6 +516,8 @@ static int parse_args(int argc, char **argv, int cmd, struct l2tp_parm *p)
>>>> /* Defaults */
>>>> p->l2spec_type = L2TP_L2SPECTYPE_DEFAULT;
>>>> p->l2spec_len = 4;
>>>> + p->udp6_csum_rx = 1;
>>>> + p->udp6_csum_tx = 1;
>>>>
>>>> while (argc > 0) {
>>>> if (strcmp(*argv, "encap") == 0) {
>>>> @@ -569,6 +587,33 @@ static int parse_args(int argc, char **argv, int cmd, struct l2tp_parm *p)
>>>> if (get_u16(&uval, *argv, 0))
>>>> invarg("invalid port\n", *argv);
>>>> p->peer_udp_port = uval;
>>>> + } else if (strcmp(*argv, "udp_csum") == 0) {
>>>> + NEXT_ARG();
>>>> + if (strcmp(*argv, "on") == 0) {
>>>> + p->udp_csum = 1;
>>>> + } else if (strcmp(*argv, "off") == 0) {
>>>> + p->udp_csum = 0;
>>>> + } else {
>>>> + invarg("invalid option for udp_csum\n", *argv);
>>>> + }
>>>> + } else if (strcmp(*argv, "udp6_csum_rx") == 0) {
>>>> + NEXT_ARG();
>>>> + if (strcmp(*argv, "on") == 0) {
>>>> + p->udp6_csum_rx = 1;
>>>> + } else if (strcmp(*argv, "off") == 0) {
>>>> + p->udp6_csum_rx = 0;
>>>> + } else {
>>>> + invarg("invalid option for udp6_csum_rx\n", *argv);
>>>> + }
>>>> + } else if (strcmp(*argv, "udp6_csum_tx") == 0) {
>>>> + NEXT_ARG();
>>>> + if (strcmp(*argv, "on") == 0) {
>>>> + p->udp6_csum_tx = 1;
>>>> + } else if (strcmp(*argv, "off") == 0) {
>>>> + p->udp6_csum_tx = 0;
>>>> + } else {
>>>> + invarg("invalid option for udp6_csum_tx\n", *argv);
>>>> + }
>>>> } else if (strcmp(*argv, "offset") == 0) {
>>>> __u8 uval;
>>>>
>>>> --
>>>> 2.5.2
>
^ permalink raw reply
* Re: [PATCH net-next] vxlan: fix initialization with custom link parameters
From: Nicolas Dichtel @ 2016-04-28 15:06 UTC (permalink / raw)
To: Jiri Benc, netdev
In-Reply-To: <7c19200582d6f4272cee914934f428775e405e89.1461854177.git.jbenc@redhat.com>
Le 28/04/2016 16:36, Jiri Benc a écrit :
> Commit 0c867c9bf84c ("vxlan: move Ethernet initialization to a separate
> function") changed initialization order and as an unintended result, when the
> user specifies additional link parameters (such as IFLA_ADDRESS) while
> creating vxlan interface, those are overwritten by vxlan_ether_setup later.
>
> It's necessary to call ether_setup from withing the ->setup callback. That
> way, the correct parameters are set by rtnl_create_link later. This is done
> also for VXLAN-GPE, as we don't know the interface type yet at that point,
> and changed to the correct interface type later.
>
> Fixes: 0c867c9bf84c ("vxlan: move Ethernet initialization to a separate function")
> Reported-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> Signed-off-by: Jiri Benc <jbenc@redhat.com>
Tested-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
^ permalink raw reply
* Re: [PATCH] net: macb: do not scan PHYs manually
From: Josh Cartwright @ 2016-04-28 15:07 UTC (permalink / raw)
To: Nathan Sullivan; +Cc: nicolas.ferre, netdev, linux-kernel
In-Reply-To: <1461853187-31715-1-git-send-email-nathan.sullivan@ni.com>
[-- Attachment #1: Type: text/plain, Size: 1493 bytes --]
On Thu, Apr 28, 2016 at 09:19:47AM -0500, Nathan Sullivan wrote:
> Since of_mdiobus_register and mdiobus_register will scan automatically,
This is only partially true. of_mdiobus_register() only scans for PHYs
with device tree presence (starting with nodes which specify an address,
then continuing for nodes without an address specifier).
So, this patch will regress any platform currently relying on a PHY
being found/probed even though it has no representation as a device tree
node. I don't have enough history with this driver to make a judgement
as to whether or not there are any users of this functionality.
> do not manually scan for PHY devices in the macb ethernet driver. Doing
> so will result in many nonexistent PHYs on the MDIO bus if the MDIO
> lines are floating or grounded, such as when they are not used.
Just a restatement/elaboration of the problem in an attempt to provide
more context:
The assumption being made is that the phy associated with a given macb
instance exists on that instance's controlled mdio bus. This assumption
doesn't hold true on some dual-ethernet Zynq 7000-series based designs
where both phys exist on the same MDIO bus (for pinning reasons, or
whatever).
MDIO
macb0 ----+----- phy0, addrX
|
+----- phy1, addrY
macb1 ----- {MDIO lines NC, not pinmuxed}
(In this case, the phy associated with macb1 is phy1, which exists on
the MDIO bus controlled by the macb0 instance).
HTH,
Josh
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply
* Re: [PATCH net-next] vxlan: fix ethernet address initialization
From: Nicolas Dichtel @ 2016-04-28 15:07 UTC (permalink / raw)
To: davem; +Cc: netdev, Jiri Benc
In-Reply-To: <1461837856-4316-1-git-send-email-nicolas.dichtel@6wind.com>
Le 28/04/2016 12:04, Nicolas Dichtel a écrit :
> Since commit 0c867c9bf84c, when the user specifies an ethernet address with
> IFLA_ADDRESS, it's overridden by vxlan_ether_setup() (rtnl_link_ops->setup
> is called in rtnetlink.c before handling IFLA_ADDRESS).
>
> To test it:
> ip link add name vxlan1 address de:ad:de:4c:0f:c2 type vxlan id 1 group 239.0.0.10 dev eth0
>
> CC: Jiri Benc <jbenc@redhat.com>
> Fixes: 0c867c9bf84c ("vxlan: move Ethernet initialization to a separate function")
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
David, please drop this patch. Jiri has sent a better fix.
Thank you,
Nicolas
^ permalink raw reply
* Re: [PATCH v2] net: macb: do not scan PHYs manually
From: Nicolas Ferre @ 2016-04-28 15:44 UTC (permalink / raw)
To: Nathan Sullivan, netdev; +Cc: linux-kernel, Florian Fainelli, Alexandre Belloni
In-Reply-To: <1461854802-8142-1-git-send-email-nathan.sullivan@ni.com>
Le 28/04/2016 16:46, Nathan Sullivan a écrit :
> Since of_mdiobus_register and mdiobus_register will scan automatically,
> do not manually scan for PHY devices in the macb ethernet driver. Doing
> so will result in many nonexistent PHYs on the MDIO bus if the MDIO
> lines are floating or grounded, such as when they are not used.
>
> Signed-off-by: Nathan Sullivan <nathan.sullivan@ni.com>
Well, as explained in the commit message that added this feature and in
the comment, if no phy is specified in the DT we end up without phy...
There are AT91 platforms which lack specification for the phy node in
the DT. So, I don't know if there is a better way to deal with this case
but I see this removal as risky.
Bye,
> ---
> drivers/net/ethernet/cadence/macb.c | 19 +------------------
> 1 file changed, 1 insertion(+), 18 deletions(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
> index 48a7d7d..6506b4e 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -424,7 +424,7 @@ static int macb_mii_init(struct macb *bp)
> {
> struct macb_platform_data *pdata;
> struct device_node *np;
> - int err = -ENXIO, i;
> + int err = -ENXIO;
>
> /* Enable management port */
> macb_writel(bp, NCR, MACB_BIT(MPE));
> @@ -450,23 +450,6 @@ static int macb_mii_init(struct macb *bp)
> if (np) {
> /* try dt phy registration */
> err = of_mdiobus_register(bp->mii_bus, np);
> -
> - /* fallback to standard phy registration if no phy were
> - found during dt phy registration */
> - if (!err && !phy_find_first(bp->mii_bus)) {
> - for (i = 0; i < PHY_MAX_ADDR; i++) {
> - struct phy_device *phydev;
> -
> - phydev = mdiobus_scan(bp->mii_bus, i);
> - if (IS_ERR(phydev)) {
> - err = PTR_ERR(phydev);
> - break;
> - }
> - }
> -
> - if (err)
> - goto err_out_unregister_bus;
> - }
> } else {
> if (pdata)
> bp->mii_bus->phy_mask = pdata->phy_mask;
>
--
Nicolas Ferre
^ permalink raw reply
* [PATCH net-next v2 0/5] bridge: per-vlan stats
From: Nikolay Aleksandrov @ 2016-04-28 15:52 UTC (permalink / raw)
To: netdev; +Cc: roopa, davem, stephen, jhs, Nikolay Aleksandrov
Hi,
This set adds support for bridge per-vlan statistics.
In order to be able to dump statistics for many vlans we need a way to
continue dumping after reaching maximum size, thus patches 01 and 02 extend
the new stats API with a per-device extended link stats attribute and
callback which can save its local state and continue where it left off
afterwards. I considered using the already existing "fill_xstats" callback
but it gets confusing since we need to separate the linkinfo dump from the
new stats api dump and adding a flag/argument to do that just looks messy.
I don't think the rtnl_link_ops size is an issue, so adding these seemed
like the cleaner approach.
Patch 03 converts the pvid to a pointer so we can consolidate the vlan
stats accounting paths later, also allows to simplify the pvid code.
Patches 04 and 05 add the stats support and netlink dump support
respectively.
I've tested this set with both old and modified iproute2, kmemleak on and
some traffic stress tests while adding/removing vlans and ports.
v2:
- Improve the error checking, rename lidx to prividx and save the current
idx user instead of restricting it to one in patch 01
- squash patch 02 into 01 and remove the restriction
- add callback descriptions, improve the size calculation and change the
xstats message structure to have an embedding level per rtnl link type
so we can avoid one call to get the link type (and thus filter on it)
and also each link type can now have any number of private attributes
inside
- fix a problem where the vlan stats are not dumped if the bridge has 0
vlans on it but has vlans on the ports, add bridge link type private
attributes and also add paddings for future extensions to avoid at least
a few netlink attributes and improve struct alignment
- drop the is_skb_forwardable argument constifying patch as it's not
needed anymore, but it's a nice cleanup which I'll send separately
Thank you,
Nik
Note: Jamal I haven't forgotten about the per-port per-vlan stats, I've got
a follow-up patch that adds it. You can easily see that the infrastructure
for private port/vlan stats is in place after this set. Though the stats
api will need some more changes to support that.
Nikolay Aleksandrov (5):
net: rtnetlink: allow rtnl_fill_statsinfo to save private state
counter
net: rtnetlink: add linkxstats callbacks and attribute
bridge: vlan: RCUify pvid
bridge: vlan: learn to count
bridge: netlink: export per-vlan stats
include/net/rtnetlink.h | 7 +++
include/uapi/linux/if_bridge.h | 18 ++++++
include/uapi/linux/if_link.h | 13 ++++
net/bridge/br_netlink.c | 88 +++++++++++++++++++++++----
net/bridge/br_private.h | 32 +++++-----
net/bridge/br_vlan.c | 134 +++++++++++++++++++++++++++++------------
net/core/rtnetlink.c | 74 +++++++++++++++++++----
7 files changed, 287 insertions(+), 79 deletions(-)
--
2.4.11
^ permalink raw reply
* [PATCH net-next v2 1/5] net: rtnetlink: allow rtnl_fill_statsinfo to save private state counter
From: Nikolay Aleksandrov @ 2016-04-28 15:52 UTC (permalink / raw)
To: netdev; +Cc: roopa, davem, stephen, jhs, Nikolay Aleksandrov
In-Reply-To: <1461858771-4732-1-git-send-email-nikolay@cumulusnetworks.com>
The new prividx argument allows the current dumping device to save a
private state counter which would enable it to continue dumping from
where it left off. And the idxattr is used to save the current idx user
so multiple prividx using attributes can be requested at the same time
as suggested by Roopa Prabhu.
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
v2: improve the error check in rtnl_fill_statsinfo, rename lidx to
prividx, squash patch 2 into this one and save the current idx user
instead of restricting only one
net/core/rtnetlink.c | 44 +++++++++++++++++++++++++++++++-------------
1 file changed, 31 insertions(+), 13 deletions(-)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 5503dfe6a050..de529a20cd18 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -3444,13 +3444,21 @@ out:
return err;
}
+static bool stats_attr_valid(unsigned int mask, int attrid, int idxattr)
+{
+ return (mask & IFLA_STATS_FILTER_BIT(attrid)) &&
+ (!idxattr || idxattr == attrid);
+}
+
static int rtnl_fill_statsinfo(struct sk_buff *skb, struct net_device *dev,
int type, u32 pid, u32 seq, u32 change,
- unsigned int flags, unsigned int filter_mask)
+ unsigned int flags, unsigned int filter_mask,
+ int *idxattr, int *prividx)
{
struct if_stats_msg *ifsm;
struct nlmsghdr *nlh;
struct nlattr *attr;
+ int s_prividx = *prividx;
ASSERT_RTNL();
@@ -3462,7 +3470,7 @@ static int rtnl_fill_statsinfo(struct sk_buff *skb, struct net_device *dev,
ifsm->ifindex = dev->ifindex;
ifsm->filter_mask = filter_mask;
- if (filter_mask & IFLA_STATS_FILTER_BIT(IFLA_STATS_LINK_64)) {
+ if (stats_attr_valid(filter_mask, IFLA_STATS_LINK_64, *idxattr)) {
struct rtnl_link_stats64 *sp;
attr = nla_reserve_64bit(skb, IFLA_STATS_LINK_64,
@@ -3480,7 +3488,11 @@ static int rtnl_fill_statsinfo(struct sk_buff *skb, struct net_device *dev,
return 0;
nla_put_failure:
- nlmsg_cancel(skb, nlh);
+ /* not a multi message or no progress mean a real error */
+ if (!(flags & NLM_F_MULTI) || s_prividx == *prividx)
+ nlmsg_cancel(skb, nlh);
+ else
+ nlmsg_end(skb, nlh);
return -EMSGSIZE;
}
@@ -3494,7 +3506,7 @@ static size_t if_nlmsg_stats_size(const struct net_device *dev,
{
size_t size = 0;
- if (filter_mask & IFLA_STATS_FILTER_BIT(IFLA_STATS_LINK_64))
+ if (stats_attr_valid(filter_mask, IFLA_STATS_LINK_64, 0))
size += nla_total_size_64bit(sizeof(struct rtnl_link_stats64));
return size;
@@ -3503,8 +3515,9 @@ static size_t if_nlmsg_stats_size(const struct net_device *dev,
static int rtnl_stats_get(struct sk_buff *skb, struct nlmsghdr *nlh)
{
struct net *net = sock_net(skb->sk);
- struct if_stats_msg *ifsm;
struct net_device *dev = NULL;
+ int idxattr = 0, prividx = 0;
+ struct if_stats_msg *ifsm;
struct sk_buff *nskb;
u32 filter_mask;
int err;
@@ -3528,7 +3541,7 @@ static int rtnl_stats_get(struct sk_buff *skb, struct nlmsghdr *nlh)
err = rtnl_fill_statsinfo(nskb, dev, RTM_NEWSTATS,
NETLINK_CB(skb).portid, nlh->nlmsg_seq, 0,
- 0, filter_mask);
+ 0, filter_mask, &idxattr, &prividx);
if (err < 0) {
/* -EMSGSIZE implies BUG in if_nlmsg_stats_size */
WARN_ON(err == -EMSGSIZE);
@@ -3542,18 +3555,19 @@ static int rtnl_stats_get(struct sk_buff *skb, struct nlmsghdr *nlh)
static int rtnl_stats_dump(struct sk_buff *skb, struct netlink_callback *cb)
{
+ int h, s_h, err, s_idx, s_idxattr, s_prividx;
struct net *net = sock_net(skb->sk);
+ unsigned int flags = NLM_F_MULTI;
struct if_stats_msg *ifsm;
- int h, s_h;
- int idx = 0, s_idx;
- struct net_device *dev;
struct hlist_head *head;
- unsigned int flags = NLM_F_MULTI;
+ struct net_device *dev;
u32 filter_mask = 0;
- int err;
+ int idx = 0;
s_h = cb->args[0];
s_idx = cb->args[1];
+ s_idxattr = cb->args[2];
+ s_prividx = cb->args[3];
cb->seq = net->dev_base_seq;
@@ -3571,7 +3585,8 @@ static int rtnl_stats_dump(struct sk_buff *skb, struct netlink_callback *cb)
err = rtnl_fill_statsinfo(skb, dev, RTM_NEWSTATS,
NETLINK_CB(cb->skb).portid,
cb->nlh->nlmsg_seq, 0,
- flags, filter_mask);
+ flags, filter_mask,
+ &s_idxattr, &s_prividx);
/* If we ran out of room on the first message,
* we're in trouble
*/
@@ -3579,13 +3594,16 @@ static int rtnl_stats_dump(struct sk_buff *skb, struct netlink_callback *cb)
if (err < 0)
goto out;
-
+ s_prividx = 0;
+ s_idxattr = 0;
nl_dump_check_consistent(cb, nlmsg_hdr(skb));
cont:
idx++;
}
}
out:
+ cb->args[3] = s_prividx;
+ cb->args[2] = s_idxattr;
cb->args[1] = idx;
cb->args[0] = h;
--
2.4.11
^ permalink raw reply related
* [PATCH net-next v2 2/5] net: rtnetlink: add linkxstats callbacks and attribute
From: Nikolay Aleksandrov @ 2016-04-28 15:52 UTC (permalink / raw)
To: netdev; +Cc: roopa, davem, stephen, jhs, Nikolay Aleksandrov
In-Reply-To: <1461858771-4732-1-git-send-email-nikolay@cumulusnetworks.com>
Add callbacks to calculate the size and fill link extended statistics
which can be split into multiple messages and are dumped via the new
rtnl stats API (RTM_GETSTATS) with the IFLA_STATS_LINK_XSTATS attribute.
Also add that attribute to the idx mask check since it is expected to
be able to save state and resume dumping (e.g. future bridge per-vlan
stats will be dumped via this attribute and callbacks).
Each link type should nest its private attributes under the per-link type
attribute. This allows to have any number of separated private attributes
and to avoid one call to get the dev link type.
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
v2: add callback descriptions and make the size calculation more
accurate, change the netlink xstats message structure with one more
level for each rtnl link type which allows for private link type attributes
and also allows us to avoid 1 call to get the dev link type.
include/net/rtnetlink.h | 7 +++++++
include/uapi/linux/if_link.h | 12 ++++++++++++
net/core/rtnetlink.c | 30 ++++++++++++++++++++++++++++++
3 files changed, 49 insertions(+)
diff --git a/include/net/rtnetlink.h b/include/net/rtnetlink.h
index 2f87c1ba13de..006a7b81d758 100644
--- a/include/net/rtnetlink.h
+++ b/include/net/rtnetlink.h
@@ -47,6 +47,9 @@ static inline int rtnl_msg_family(const struct nlmsghdr *nlh)
* @get_num_rx_queues: Function to determine number of receive queues
* to create when creating a new device.
* @get_link_net: Function to get the i/o netns of the device
+ * @get_linkxstats_size: Function to calculate the required room for
+ * dumping device-specific extended link stats
+ * @fill_linkxstats: Function to dump device-specific extended link stats
*/
struct rtnl_link_ops {
struct list_head list;
@@ -95,6 +98,10 @@ struct rtnl_link_ops {
const struct net_device *dev,
const struct net_device *slave_dev);
struct net *(*get_link_net)(const struct net_device *dev);
+ size_t (*get_linkxstats_size)(const struct net_device *dev);
+ int (*fill_linkxstats)(struct sk_buff *skb,
+ const struct net_device *dev,
+ int *prividx);
};
int __rtnl_link_register(struct rtnl_link_ops *ops);
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index d82de331bb6b..8577c0e4116f 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -802,6 +802,7 @@ struct if_stats_msg {
enum {
IFLA_STATS_UNSPEC, /* also used as 64bit pad attribute */
IFLA_STATS_LINK_64,
+ IFLA_STATS_LINK_XSTATS,
__IFLA_STATS_MAX,
};
@@ -809,4 +810,15 @@ enum {
#define IFLA_STATS_FILTER_BIT(ATTR) (1 << (ATTR - 1))
+/* These are embedded into IFLA_STATS_LINK_XSTATS:
+ * [IFLA_STATS_LINK_XSTATS]
+ * -> [LINK_XSTATS_TYPE_xxx]
+ * -> [rtnl link type specific attributes]
+ */
+enum {
+ LINK_XSTATS_TYPE_UNSPEC,
+ __LINK_XSTATS_TYPE_MAX
+};
+#define LINK_XSTATS_TYPE_MAX (__LINK_XSTATS_TYPE_MAX - 1)
+
#endif /* _UAPI_LINUX_IF_LINK_H */
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index de529a20cd18..d471f097c739 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -3483,6 +3483,26 @@ static int rtnl_fill_statsinfo(struct sk_buff *skb, struct net_device *dev,
dev_get_stats(dev, sp);
}
+ if (stats_attr_valid(filter_mask, IFLA_STATS_LINK_XSTATS, *idxattr)) {
+ const struct rtnl_link_ops *ops = dev->rtnl_link_ops;
+
+ if (ops && ops->fill_linkxstats) {
+ int err;
+
+ *idxattr = IFLA_STATS_LINK_XSTATS;
+ attr = nla_nest_start(skb,
+ IFLA_STATS_LINK_XSTATS);
+ if (!attr)
+ goto nla_put_failure;
+
+ err = ops->fill_linkxstats(skb, dev, prividx);
+ nla_nest_end(skb, attr);
+ if (err)
+ goto nla_put_failure;
+ *idxattr = 0;
+ }
+ }
+
nlmsg_end(skb, nlh);
return 0;
@@ -3509,6 +3529,16 @@ static size_t if_nlmsg_stats_size(const struct net_device *dev,
if (stats_attr_valid(filter_mask, IFLA_STATS_LINK_64, 0))
size += nla_total_size_64bit(sizeof(struct rtnl_link_stats64));
+ if (stats_attr_valid(filter_mask, IFLA_STATS_LINK_XSTATS, 0)) {
+ const struct rtnl_link_ops *ops = dev->rtnl_link_ops;
+
+ if (ops && ops->get_linkxstats_size) {
+ size += nla_total_size(ops->get_linkxstats_size(dev));
+ /* for IFLA_STATS_LINK_XSTATS */
+ size += nla_total_size(0);
+ }
+ }
+
return size;
}
--
2.4.11
^ permalink raw reply related
* [PATCH net-next v2 3/5] bridge: vlan: RCUify pvid
From: Nikolay Aleksandrov @ 2016-04-28 15:52 UTC (permalink / raw)
To: netdev; +Cc: roopa, davem, stephen, jhs, Nikolay Aleksandrov
In-Reply-To: <1461858771-4732-1-git-send-email-nikolay@cumulusnetworks.com>
Make pvid a pointer to a vlan struct and RCUify the access to it. Vlans
are already RCU-protected so the pvid vlan entry cannot disappear
without being initialized to NULL and going through a grace period first.
This change is necessary for the upcoming vlan counters and also would
serve to later move to vlan passing via a pointer instead of id.
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
v2: no change
net/bridge/br_netlink.c | 23 ++++++++++-----------
net/bridge/br_private.h | 16 +--------------
net/bridge/br_vlan.c | 54 +++++++++++++++++++++++--------------------------
3 files changed, 37 insertions(+), 56 deletions(-)
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 6bae1125e36d..9c74fa438a8c 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -24,22 +24,22 @@
static int __get_num_vlan_infos(struct net_bridge_vlan_group *vg,
u32 filter_mask)
{
- struct net_bridge_vlan *v;
+ struct net_bridge_vlan *v, *pvid;
u16 vid_range_start = 0, vid_range_end = 0, vid_range_flags = 0;
- u16 flags, pvid;
int num_vlans = 0;
+ u16 flags;
if (!(filter_mask & RTEXT_FILTER_BRVLAN_COMPRESSED))
return 0;
- pvid = br_get_pvid(vg);
+ pvid = rcu_dereference(vg->pvid);
/* Count number of vlan infos */
list_for_each_entry_rcu(v, &vg->vlan_list, vlist) {
flags = 0;
/* only a context, bridge vlan not activated */
if (!br_vlan_should_use(v))
continue;
- if (v->vid == pvid)
+ if (v == pvid)
flags |= BRIDGE_VLAN_INFO_PVID;
if (v->flags & BRIDGE_VLAN_INFO_UNTAGGED)
@@ -246,21 +246,21 @@ nla_put_failure:
static int br_fill_ifvlaninfo_compressed(struct sk_buff *skb,
struct net_bridge_vlan_group *vg)
{
- struct net_bridge_vlan *v;
+ struct net_bridge_vlan *v, *pvid;
u16 vid_range_start = 0, vid_range_end = 0, vid_range_flags = 0;
- u16 flags, pvid;
int err = 0;
+ u16 flags;
/* Pack IFLA_BRIDGE_VLAN_INFO's for every vlan
* and mark vlan info with begin and end flags
* if vlaninfo represents a range
*/
- pvid = br_get_pvid(vg);
+ pvid = rcu_dereference(vg->pvid);
list_for_each_entry_rcu(v, &vg->vlan_list, vlist) {
flags = 0;
if (!br_vlan_should_use(v))
continue;
- if (v->vid == pvid)
+ if (v == pvid)
flags |= BRIDGE_VLAN_INFO_PVID;
if (v->flags & BRIDGE_VLAN_INFO_UNTAGGED)
@@ -301,18 +301,17 @@ initvars:
static int br_fill_ifvlaninfo(struct sk_buff *skb,
struct net_bridge_vlan_group *vg)
{
+ struct net_bridge_vlan *v, *pvid;
struct bridge_vlan_info vinfo;
- struct net_bridge_vlan *v;
- u16 pvid;
- pvid = br_get_pvid(vg);
+ pvid = rcu_dereference(vg->pvid);
list_for_each_entry_rcu(v, &vg->vlan_list, vlist) {
if (!br_vlan_should_use(v))
continue;
vinfo.vid = v->vid;
vinfo.flags = 0;
- if (v->vid == pvid)
+ if (v == pvid)
vinfo.flags |= BRIDGE_VLAN_INFO_PVID;
if (v->flags & BRIDGE_VLAN_INFO_UNTAGGED)
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 1b5d145dfcbf..50d70b5eb307 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -130,8 +130,8 @@ struct net_bridge_vlan {
struct net_bridge_vlan_group {
struct rhashtable vlan_hash;
struct list_head vlan_list;
+ struct net_bridge_vlan __rcu *pvid;
u16 num_vlans;
- u16 pvid;
};
struct net_bridge_fdb_entry
@@ -741,15 +741,6 @@ static inline int br_vlan_get_tag(const struct sk_buff *skb, u16 *vid)
return err;
}
-static inline u16 br_get_pvid(const struct net_bridge_vlan_group *vg)
-{
- if (!vg)
- return 0;
-
- smp_rmb();
- return vg->pvid;
-}
-
static inline int br_vlan_enabled(struct net_bridge *br)
{
return br->vlan_enabled;
@@ -835,11 +826,6 @@ static inline u16 br_vlan_get_tag(const struct sk_buff *skb, u16 *tag)
return 0;
}
-static inline u16 br_get_pvid(const struct net_bridge_vlan_group *vg)
-{
- return 0;
-}
-
static inline int br_vlan_enabled(struct net_bridge *br)
{
return 0;
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index e001152d6ad1..4fab7665df8c 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -31,22 +31,11 @@ static struct net_bridge_vlan *br_vlan_lookup(struct rhashtable *tbl, u16 vid)
return rhashtable_lookup_fast(tbl, &vid, br_vlan_rht_params);
}
-static void __vlan_add_pvid(struct net_bridge_vlan_group *vg, u16 vid)
+/* __vlan_delete_pvid is just __vlan_set_pvid(vg, NULL) */
+static void __vlan_set_pvid(struct net_bridge_vlan_group *vg,
+ struct net_bridge_vlan *v)
{
- if (vg->pvid == vid)
- return;
-
- smp_wmb();
- vg->pvid = vid;
-}
-
-static void __vlan_delete_pvid(struct net_bridge_vlan_group *vg, u16 vid)
-{
- if (vg->pvid != vid)
- return;
-
- smp_wmb();
- vg->pvid = 0;
+ rcu_assign_pointer(vg->pvid, v);
}
static void __vlan_add_flags(struct net_bridge_vlan *v, u16 flags)
@@ -59,9 +48,9 @@ static void __vlan_add_flags(struct net_bridge_vlan *v, u16 flags)
vg = nbp_vlan_group(v->port);
if (flags & BRIDGE_VLAN_INFO_PVID)
- __vlan_add_pvid(vg, v->vid);
- else
- __vlan_delete_pvid(vg, v->vid);
+ __vlan_set_pvid(vg, v);
+ else if (rtnl_dereference(vg->pvid) == v)
+ __vlan_set_pvid(vg, NULL);
if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
v->flags |= BRIDGE_VLAN_INFO_UNTAGGED;
@@ -285,7 +274,9 @@ static int __vlan_del(struct net_bridge_vlan *v)
masterv = v->brvlan;
}
- __vlan_delete_pvid(vg, v->vid);
+ if (rtnl_dereference(vg->pvid) == v)
+ __vlan_set_pvid(vg, NULL);
+
if (p) {
err = __vlan_vid_del(p->dev, p->br, v->vid);
if (err)
@@ -320,7 +311,7 @@ static void __vlan_flush(struct net_bridge_vlan_group *vg)
{
struct net_bridge_vlan *vlan, *tmp;
- __vlan_delete_pvid(vg, vg->pvid);
+ __vlan_set_pvid(vg, NULL);
list_for_each_entry_safe(vlan, tmp, &vg->vlan_list, vlist)
__vlan_del(vlan);
}
@@ -404,29 +395,29 @@ static bool __allowed_ingress(struct net_bridge_vlan_group *vg, __be16 proto,
}
if (!*vid) {
- u16 pvid = br_get_pvid(vg);
+ v = rcu_dereference(vg->pvid);
/* Frame had a tag with VID 0 or did not have a tag.
* See if pvid is set on this port. That tells us which
* vlan untagged or priority-tagged traffic belongs to.
*/
- if (!pvid)
+ if (!v)
goto drop;
/* PVID is set on this port. Any untagged or priority-tagged
* ingress frame is considered to belong to this vlan.
*/
- *vid = pvid;
+ *vid = v->vid;
if (likely(!tagged))
/* Untagged Frame. */
- __vlan_hwaccel_put_tag(skb, proto, pvid);
+ __vlan_hwaccel_put_tag(skb, proto, v->vid);
else
/* Priority-tagged Frame.
* At this point, We know that skb->vlan_tci had
* VLAN_TAG_PRESENT bit and its VID field was 0x000.
* We update only VID field and preserve PCP field.
*/
- skb->vlan_tci |= pvid;
+ skb->vlan_tci |= v->vid;
return true;
}
@@ -451,6 +442,9 @@ bool br_allowed_ingress(const struct net_bridge *br,
BR_INPUT_SKB_CB(skb)->vlan_filtered = false;
return true;
}
+ /* if there's no vlan_group, there's nothing to match against */
+ if (!vg)
+ return false;
return __allowed_ingress(vg, br->vlan_proto, skb, vid);
}
@@ -492,9 +486,11 @@ bool br_should_learn(struct net_bridge_port *p, struct sk_buff *skb, u16 *vid)
*vid = 0;
if (!*vid) {
- *vid = br_get_pvid(vg);
- if (!*vid)
+ struct net_bridge_vlan *v = rcu_dereference(vg->pvid);
+
+ if (!v)
return false;
+ *vid = v->vid;
return true;
}
@@ -713,9 +709,9 @@ int br_vlan_set_proto(struct net_bridge *br, unsigned long val)
static bool vlan_default_pvid(struct net_bridge_vlan_group *vg, u16 vid)
{
- struct net_bridge_vlan *v;
+ struct net_bridge_vlan *v = rtnl_dereference(vg->pvid);
- if (vid != vg->pvid)
+ if (v && vid != v->vid)
return false;
v = br_vlan_lookup(&vg->vlan_hash, vid);
--
2.4.11
^ permalink raw reply related
* [PATCH net-next v2 4/5] bridge: vlan: learn to count
From: Nikolay Aleksandrov @ 2016-04-28 15:52 UTC (permalink / raw)
To: netdev; +Cc: roopa, davem, stephen, jhs, Nikolay Aleksandrov
In-Reply-To: <1461858771-4732-1-git-send-email-nikolay@cumulusnetworks.com>
Add support for per-VLAN Tx/Rx statistics. Every global vlan context gets
allocated a per-cpu stats which is then set in each per-port vlan context
for quick access. The br_allowed_ingress() common function is used to
account for Rx packets and the br_handle_vlan() common function is used
to account for Tx packets.
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
v2: no change
net/bridge/br_private.h | 11 +++++++++-
net/bridge/br_vlan.c | 53 +++++++++++++++++++++++++++++++++++++++----------
2 files changed, 53 insertions(+), 11 deletions(-)
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 50d70b5eb307..f6876ed718a5 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -77,12 +77,21 @@ struct bridge_mcast_querier {
};
#endif
+struct br_vlan_stats {
+ u64 rx_bytes;
+ u64 rx_packets;
+ u64 tx_bytes;
+ u64 tx_packets;
+ struct u64_stats_sync syncp;
+};
+
/**
* struct net_bridge_vlan - per-vlan entry
*
* @vnode: rhashtable member
* @vid: VLAN id
* @flags: bridge vlan flags
+ * @stats: per-cpu VLAN statistics
* @br: if MASTER flag set, this points to a bridge struct
* @port: if MASTER flag unset, this points to a port struct
* @refcnt: if MASTER flag set, this is bumped for each port referencing it
@@ -100,6 +109,7 @@ struct net_bridge_vlan {
struct rhash_head vnode;
u16 vid;
u16 flags;
+ struct br_vlan_stats __percpu *stats;
union {
struct net_bridge *br;
struct net_bridge_port *port;
@@ -866,7 +876,6 @@ static inline struct net_bridge_vlan_group *nbp_vlan_group_rcu(
{
return NULL;
}
-
#endif
struct nf_br_ops {
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 4fab7665df8c..d7a70c2ea3ec 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -151,6 +151,17 @@ static struct net_bridge_vlan *br_vlan_get_master(struct net_bridge *br, u16 vid
return masterv;
}
+static void br_master_vlan_rcu_free(struct rcu_head *rcu)
+{
+ struct net_bridge_vlan *v;
+
+ v = container_of(rcu, struct net_bridge_vlan, rcu);
+ WARN_ON(!br_vlan_is_master(v));
+ free_percpu(v->stats);
+ v->stats = NULL;
+ kfree(v);
+}
+
static void br_vlan_put_master(struct net_bridge_vlan *masterv)
{
struct net_bridge_vlan_group *vg;
@@ -163,7 +174,7 @@ static void br_vlan_put_master(struct net_bridge_vlan *masterv)
rhashtable_remove_fast(&vg->vlan_hash,
&masterv->vnode, br_vlan_rht_params);
__vlan_del_list(masterv);
- kfree_rcu(masterv, rcu);
+ call_rcu(&masterv->rcu, br_master_vlan_rcu_free);
}
}
@@ -219,6 +230,7 @@ static int __vlan_add(struct net_bridge_vlan *v, u16 flags)
if (!masterv)
goto out_filt;
v->brvlan = masterv;
+ v->stats = masterv->stats;
}
/* Add the dev mac and count the vlan only if it's usable */
@@ -320,6 +332,7 @@ struct sk_buff *br_handle_vlan(struct net_bridge *br,
struct net_bridge_vlan_group *vg,
struct sk_buff *skb)
{
+ struct br_vlan_stats *stats;
struct net_bridge_vlan *v;
u16 vid;
@@ -346,9 +359,14 @@ struct sk_buff *br_handle_vlan(struct net_bridge *br,
return NULL;
}
}
+ stats = this_cpu_ptr(v->stats);
+ u64_stats_update_begin(&stats->syncp);
+ stats->tx_bytes += skb->len;
+ stats->tx_packets++;
+ u64_stats_update_end(&stats->syncp);
+
if (v->flags & BRIDGE_VLAN_INFO_UNTAGGED)
skb->vlan_tci = 0;
-
out:
return skb;
}
@@ -357,7 +375,8 @@ out:
static bool __allowed_ingress(struct net_bridge_vlan_group *vg, __be16 proto,
struct sk_buff *skb, u16 *vid)
{
- const struct net_bridge_vlan *v;
+ struct br_vlan_stats *stats;
+ struct net_bridge_vlan *v;
bool tagged;
BR_INPUT_SKB_CB(skb)->vlan_filtered = true;
@@ -418,14 +437,21 @@ static bool __allowed_ingress(struct net_bridge_vlan_group *vg, __be16 proto,
* We update only VID field and preserve PCP field.
*/
skb->vlan_tci |= v->vid;
-
- return true;
+ } else {
+ /* Frame had a valid vlan tag. See if vlan is allowed */
+ v = br_vlan_find(vg, *vid);
}
+ if (!v || !br_vlan_should_use(v))
+ goto drop;
+
+ stats = this_cpu_ptr(v->stats);
+ u64_stats_update_begin(&stats->syncp);
+ stats->rx_bytes += skb->len;
+ stats->rx_packets++;
+ u64_stats_update_end(&stats->syncp);
+
+ return true;
- /* Frame had a valid vlan tag. See if vlan is allowed */
- v = br_vlan_find(vg, *vid);
- if (v && br_vlan_should_use(v))
- return true;
drop:
kfree_skb(skb);
return false;
@@ -538,6 +564,11 @@ int br_vlan_add(struct net_bridge *br, u16 vid, u16 flags)
if (!vlan)
return -ENOMEM;
+ vlan->stats = netdev_alloc_pcpu_stats(struct br_vlan_stats);
+ if (!vlan->stats) {
+ kfree(vlan);
+ return -ENOMEM;
+ }
vlan->vid = vid;
vlan->flags = flags | BRIDGE_VLAN_INFO_MASTER;
vlan->flags &= ~BRIDGE_VLAN_INFO_PVID;
@@ -545,8 +576,10 @@ int br_vlan_add(struct net_bridge *br, u16 vid, u16 flags)
if (flags & BRIDGE_VLAN_INFO_BRENTRY)
atomic_set(&vlan->refcnt, 1);
ret = __vlan_add(vlan, flags);
- if (ret)
+ if (ret) {
+ free_percpu(vlan->stats);
kfree(vlan);
+ }
return ret;
}
--
2.4.11
^ 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