* Re: [PATCH net] bonding: fix accounting of active ports in 3ad
From: David Miller @ 2017-05-19 23:17 UTC (permalink / raw)
To: jarod; +Cc: linux-kernel, j.vosburgh, vfalico, andy, netdev
In-Reply-To: <95c9a6b9-ac60-caa1-416d-c74e1aea08f4@redhat.com>
From: Jarod Wilson <jarod@redhat.com>
Date: Fri, 19 May 2017 18:15:57 -0400
> On 2017-05-19 5:14 PM, David Miller wrote:
>> From: Jarod Wilson <jarod@redhat.com>
>> Date: Wed, 17 May 2017 11:11:44 -0400
>>
>>> As of 7bb11dc9f59d and 0622cab0341c, bond slaves in a 3ad bond are not
>>> removed from the aggregator when they are down, and the active slave
>>> count
>>> is NOT equal to number of ports in the aggregator, but rather the
>>> number
>>> of ports in the aggregator that are still enabled.
>> ...
>>> Remedy it by using the same logic introduced in
>>> 7bb11dc9f59d in __bond_3ad_get_active_agg_info(), so sysfs, procfs and
>> ^^^^^^^^^^^^
>>> netlink all report the number of active ports.
>> I think you mean to reference commit 0622cab0341c here not
>> 7bb11dc9f59d.
>
> D'oh, yes, you are entirely correct. Should I submit a v2 with that
> correction?
Yes, please.
^ permalink raw reply
* Re: [PATCH net-next v3 0/7] fix CRC32c in the forwarding path
From: David Miller @ 2017-05-19 23:22 UTC (permalink / raw)
To: dcaratti
Cc: linux-sctp, netdev, alexander.duyck, tom, David.Laight,
marcelo.leitner
In-Reply-To: <cover.1495112301.git.dcaratti@redhat.com>
From: Davide Caratti <dcaratti@redhat.com>
Date: Thu, 18 May 2017 15:44:36 +0200
> Current kernel allows offloading CRC32c computation when SCTP packets
> are generated, setting skb->ip_summed to CHECKSUM_PARTIAL, if the
> underlying device features have NETIF_F_SCTP_CRC set. However, after these
> packets are forwarded, they may land on a device where CRC32c offloading is
> not available: as a consequence, transmission is done with wrong CRC32c.
> It's not possible to use sctp_compte_cksum() in the forwarding path
> and in most drivers, because it needs symbols exported by libcrc32c module.
>
> Patch 1 and 2 of this series try to solve this problem, introducing a new
> helper function, namely skb_crc32c_csum_help(), that can be used to resolve
> CHECKSUM_PARTIAL when crc32c is needed instead of Internet Checksum.
>
> Currently, we need to parse the packet headers to understand what algorithm
> is needed to resolve CHECKSUM_PARTIAL. We can speedup things by storing
> this information in the skb metadata, and use it to call an appropriate
> helper (skb_checksum_help or skb_crc32c_csum_help), or leave the packet
> unmodified when the NIC is able to offload the checksum computation.
>
> Patch 3 deprecates skb->csum_bad to free one bit in skb metadata; patch 4
> introduces skb->csum_not_inet, providing skb with an indication on the
> algorithm needed to resolve CHECKSUM_PARTIAL.
> Patch 5 and 6 fix the kernel forwarding path and openvswitch datapath,
> where skb_checksum_help was unconditionally called to resolve CHECKSUM_PARTIAL,
> thus generating wrong CRC32c in forwarded SCTP packets.
> Finally, patch 7 updates documentation to provide a better description of
> possible values of skb->ip_summed.
>
> Some further work is still possible:
> * drivers that parse the packet header to correctly resolve CHECKSUM_PARTIAL
> (e.g. ixgbe_tx_csum()) can benefit from testing skb->csum_not_inet to avoid
> calling ip_hdr(skb)->protocol or ixgbe_ipv6_csum_is_sctp(skb).
>
> * drivers that call skb_checksum_help() to resolve CHECKSUM_PARTIAL can
> call skb_csum_hwoffload_help to avoid corrupting SCTP packets.
...
Ok, series applied.
I do kinda think that the crc32 module handling still isn't very nice.
If this is a core checksumming algorithm we support in the networking
stack, than seriously just like the standard internet checksum we should
statically build crc32 into the kernel image and not have this weird
situation where the code might not be there when we need it.
Thanks.
^ permalink raw reply
* Re: [RFC net-next PATCH 3/5] net: introduce XDP driver features interface
From: David Miller @ 2017-05-19 23:37 UTC (permalink / raw)
To: daniel; +Cc: brouer, borkmann, alexei.starovoitov, john.r.fastabend, netdev
In-Reply-To: <591F27B9.9070003@iogearbox.net>
From: Daniel Borkmann <daniel@iogearbox.net>
Date: Fri, 19 May 2017 19:13:29 +0200
> The problem is that once you add bits markers to bpf_prog like we
> used to do in the past, then as you do in patch 4/5 with the
> xdp_rxhash_needed bit, they will need to be turned /on/
> unconditionally when a prog has tail calls.
Yeah that's the problem with feature checks, once you have tail calls
involved we have to say "entire universe" of features is possible
because it is (intentionally) not possible to track all paths
reachable via tail calls, and in fact these paths can dynamically and
arbitrarily change after the program using tail calls have been loaded
and verified completely.
For example, let's assume we have eBPF program A that uses tail calls
via slots in bpf MAP "M". At verification time, sure, we could see
the MAP "M" points to programs B and C, which don't use tail calls and
look at what features they use.
But after loading "A", anyone with access to bpf MAP "M" can change
the tail call slots to point to bpf programs other than "B" and "C".
And maybe those new programs use features outside of the set we tested
for when "A" was verified.
So it is impossible to test feature "sets" with eBPF like this.
^ permalink raw reply
* Re: [RFC net-next PATCH 2/5] mlx5: fix bug reading rss_hash_type from CQE
From: David Miller @ 2017-05-19 23:38 UTC (permalink / raw)
To: brouer; +Cc: borkmann, alexei.starovoitov, john.r.fastabend, netdev
In-Reply-To: <149512209807.14733.12774004120782832472.stgit@firesoul>
From: Jesper Dangaard Brouer <brouer@redhat.com>
Date: Thu, 18 May 2017 17:41:38 +0200
> Masks for extracting part of the Completion Queue Entry (CQE)
> field rss_hash_type was swapped, namely CQE_RSS_HTYPE_IP and
> CQE_RSS_HTYPE_L4.
>
> The bug resulted in setting skb->l4_hash, even-though the
> rss_hash_type indicated that hash was NOT computed over the
> L4 (UDP or TCP) part of the packet.
>
> Added comments from the datasheet, to make it more clear what
> these masks are selecting.
>
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
Please pass this along to the mlx5 developers as a standalone
bug fix for 'net', thank you.
^ permalink raw reply
* Re: [PATCH v2 net-next 0/2] Check all RGMII phy mode variants
From: David Miller @ 2017-05-19 23:42 UTC (permalink / raw)
To: isubramanian; +Cc: netdev, f.fainelli, andrew, linux-arm-kernel, patches
In-Reply-To: <1495145624-29463-1-git-send-email-isubramanian@apm.com>
From: Iyappan Subramanian <isubramanian@apm.com>
Date: Thu, 18 May 2017 15:13:42 -0700
> This patch set,
> - adds phy_interface_mode_is_rgmii() helper function
> - addresses review comment from previous patch set, by calling
> phy_interface_mode_is_rgmii() to address all RGMII variants
>
> Signed-off-by: Iyappan Subramanian <isubramanian@apm.com>
Series applied, thanks.
^ permalink raw reply
* [PATCH net v2] bonding: fix accounting of active ports in 3ad
From: Jarod Wilson @ 2017-05-19 23:43 UTC (permalink / raw)
To: linux-kernel
Cc: Jarod Wilson, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek,
netdev
In-Reply-To: <20170517151144.57865-1-jarod@redhat.com>
As of 7bb11dc9f59d and 0622cab0341c, bond slaves in a 3ad bond are not
removed from the aggregator when they are down, and the active slave count
is NOT equal to number of ports in the aggregator, but rather the number
of ports in the aggregator that are still enabled. The sysfs spew for
bonding_show_ad_num_ports() has a comment that says "Show number of active
802.3ad ports.", but it's currently showing total number of ports, both
active and inactive. Remedy it by using the same logic introduced in
0622cab0341c in __bond_3ad_get_active_agg_info(), so sysfs, procfs and
netlink all report the number of active ports. Note that this means that
IFLA_BOND_AD_INFO_NUM_PORTS really means NUM_ACTIVE_PORTS instead of
NUM_PORTS, and thus perhaps should be renamed for clarity.
Lightly tested on a dual i40e lacp bond, simulating link downs with an ip
link set dev <slave2> down, was able to produce the state where I could
see both in the same aggregator, but a number of ports count of 1.
MII Status: up
Active Aggregator Info:
Aggregator ID: 1
Number of ports: 2 <---
Slave Interface: ens10
MII Status: up <---
Aggregator ID: 1
Slave Interface: ens11
MII Status: up
Aggregator ID: 1
MII Status: up
Active Aggregator Info:
Aggregator ID: 1
Number of ports: 1 <---
Slave Interface: ens10
MII Status: down <---
Aggregator ID: 1
Slave Interface: ens11
MII Status: up
Aggregator ID: 1
CC: Jay Vosburgh <j.vosburgh@gmail.com>
CC: Veaceslav Falico <vfalico@gmail.com>
CC: Andy Gospodarek <andy@greyhouse.net>
CC: netdev@vger.kernel.org
Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
v2: fix incorrect git sha reference, add more testing data
drivers/net/bonding/bond_3ad.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index c5fd4259da33..b44a6aeb346d 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -2577,7 +2577,7 @@ int __bond_3ad_get_active_agg_info(struct bonding *bond,
return -1;
ad_info->aggregator_id = aggregator->aggregator_identifier;
- ad_info->ports = aggregator->num_of_ports;
+ ad_info->ports = __agg_active_ports(aggregator);
ad_info->actor_key = aggregator->actor_oper_aggregator_key;
ad_info->partner_key = aggregator->partner_oper_aggregator_key;
ether_addr_copy(ad_info->partner_system,
--
2.12.1
^ permalink raw reply related
* Re: [PATCH v3 net-next 1/5] dsa: add support for Microchip KSZ tail tagging
From: Florian Fainelli @ 2017-05-20 0:02 UTC (permalink / raw)
To: Woojung.Huh, andrew, vivien.didelot, sergei.shtylyov
Cc: netdev, davem, UNGLinuxDriver
In-Reply-To: <9235D6609DB808459E95D78E17F2E43D40A7C018@CHN-SV-EXMX02.mchp-main.com>
On 05/19/2017 03:57 PM, Woojung.Huh@microchip.com wrote:
> From: Woojung Huh <Woojung.Huh@microchip.com>
>
> Adding support for the Microchip KSZ switch family tail tagging.
>
> Signed-off-by: Woojung Huh <Woojung.Huh@microchip.com>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> ---
> include/net/dsa.h | 1 +
> net/dsa/Kconfig | 3 ++
> net/dsa/Makefile | 1 +
> net/dsa/dsa.c | 3 ++
> net/dsa/dsa_priv.h | 3 ++
> net/dsa/tag_ksz.c | 103 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> 6 files changed, 114 insertions(+)
> create mode 100644 net/dsa/tag_ksz.c
>
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index 791fed6..fbb00a6 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -31,6 +31,7 @@ enum dsa_tag_protocol {
> DSA_TAG_PROTO_BRCM,
> DSA_TAG_PROTO_DSA,
> DSA_TAG_PROTO_EDSA,
> + DSA_TAG_PROTO_KSZ,
> DSA_TAG_PROTO_LAN9303,
> DSA_TAG_PROTO_MTK,
> DSA_TAG_PROTO_QCA,
> diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig
> index 297389b..cc5f8f9 100644
> --- a/net/dsa/Kconfig
> +++ b/net/dsa/Kconfig
> @@ -25,6 +25,9 @@ config NET_DSA_TAG_DSA
> config NET_DSA_TAG_EDSA
> bool
>
> +config NET_DSA_TAG_KSZ
> + bool
> +
> config NET_DSA_TAG_LAN9303
> bool
>
> diff --git a/net/dsa/Makefile b/net/dsa/Makefile
> index f8c0251..b15141f 100644
> --- a/net/dsa/Makefile
> +++ b/net/dsa/Makefile
> @@ -6,6 +6,7 @@ dsa_core-y += dsa.o slave.o dsa2.o switch.o legacy.o
> dsa_core-$(CONFIG_NET_DSA_TAG_BRCM) += tag_brcm.o
> dsa_core-$(CONFIG_NET_DSA_TAG_DSA) += tag_dsa.o
> dsa_core-$(CONFIG_NET_DSA_TAG_EDSA) += tag_edsa.o
> +dsa_core-$(CONFIG_NET_DSA_TAG_KSZ) += tag_ksz.o
> dsa_core-$(CONFIG_NET_DSA_TAG_LAN9303) += tag_lan9303.o
> dsa_core-$(CONFIG_NET_DSA_TAG_MTK) += tag_mtk.o
> dsa_core-$(CONFIG_NET_DSA_TAG_QCA) += tag_qca.o
> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
> index 3288a80..402459e 100644
> --- a/net/dsa/dsa.c
> +++ b/net/dsa/dsa.c
> @@ -49,6 +49,9 @@ const struct dsa_device_ops *dsa_device_ops[DSA_TAG_LAST] = {
> #ifdef CONFIG_NET_DSA_TAG_EDSA
> [DSA_TAG_PROTO_EDSA] = &edsa_netdev_ops,
> #endif
> +#ifdef CONFIG_NET_DSA_TAG_KSZ
> + [DSA_TAG_PROTO_KSZ] = &ksz_netdev_ops,
> +#endif
> #ifdef CONFIG_NET_DSA_TAG_LAN9303
> [DSA_TAG_PROTO_LAN9303] = &lan9303_netdev_ops,
> #endif
> diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
> index c274130..6f23dfa 100644
> --- a/net/dsa/dsa_priv.h
> +++ b/net/dsa/dsa_priv.h
> @@ -85,6 +85,9 @@ extern const struct dsa_device_ops dsa_netdev_ops;
> /* tag_edsa.c */
> extern const struct dsa_device_ops edsa_netdev_ops;
>
> +/* tag_ksz.c */
> +extern const struct dsa_device_ops ksz_netdev_ops;
> +
> /* tag_lan9303.c */
> extern const struct dsa_device_ops lan9303_netdev_ops;
>
> diff --git a/net/dsa/tag_ksz.c b/net/dsa/tag_ksz.c
> new file mode 100644
> index 0000000..cbc79b5
> --- /dev/null
> +++ b/net/dsa/tag_ksz.c
> @@ -0,0 +1,103 @@
> +/*
> + * net/dsa/tag_ksz.c - Microchip KSZ Switch tag format handling
> + * Copyright (c) 2017 Microchip Technology
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include <linux/etherdevice.h>
> +#include <linux/list.h>
> +#include <linux/slab.h>
> +#include <net/dsa.h>
> +#include "dsa_priv.h"
> +
> +/* For Ingress (Host -> KSZ), 2 bytes are added before FCS.
> + * ---------------------------------------------------------------------------
> + * DA(6bytes)|SA(6bytes)|....|Data(nbytes)|tag0(1byte)|tag1(1byte)|FCS(4bytes)
> + * ---------------------------------------------------------------------------
> + * tag0 : Prioritization (not used now)
> + * tag1 : each bit represents port (eg, 0x01=port1, 0x02=port2, 0x10=port5)
> + *
> + * For Egress (KSZ -> Host), 1 byte is added before FCS.
> + * ---------------------------------------------------------------------------
> + * DA(6bytes)|SA(6bytes)|....|Data(nbytes)|tag0(1byte)|FCS(4bytes)
> + * ---------------------------------------------------------------------------
> + * tag0 : zero-based value represents port
> + * (eg, 0x00=port1, 0x02=port3, 0x06=port7)
> + */
> +
> +#define KSZ_INGRESS_TAG_LEN 2
> +#define KSZ_EGRESS_TAG_LEN 1
> +
> +static struct sk_buff *ksz_xmit(struct sk_buff *skb, struct net_device *dev)
> +{
> + struct dsa_slave_priv *p = netdev_priv(dev);
> + struct sk_buff *nskb;
> + int padlen;
> + u8 *tag;
> +
> + padlen = (skb->len >= ETH_ZLEN) ? 0 : ETH_ZLEN - skb->len;
> +
> + if (skb_tailroom(skb) >= padlen + KSZ_INGRESS_TAG_LEN) {
> + nskb = skb;
> + } else {
> + nskb = alloc_skb(NET_IP_ALIGN + skb->len +
> + padlen + KSZ_INGRESS_TAG_LEN, GFP_ATOMIC);
> + if (!nskb) {
> + kfree_skb(skb);
> + return NULL;
> + }
> + skb_reserve(nskb, NET_IP_ALIGN);
> +
> + skb_reset_mac_header(nskb);
> + skb_set_network_header(nskb,
> + skb_network_header(skb) - skb->head);
> + skb_set_transport_header(nskb,
> + skb_transport_header(skb) - skb->head);
> + skb_copy_and_csum_dev(skb, skb_put(nskb, skb->len));
> + kfree_skb(skb);
> + }
> +
> + if (padlen) {
> + u8 *pad = skb_put(nskb, padlen);
> +
> + memset(pad, 0, padlen);
> + }
Can you use skb_put_padto() here instead of open coding this?
> +
> + tag = skb_put(nskb, KSZ_INGRESS_TAG_LEN);
> + tag[0] = 0;
> + tag[1] = 1 << p->dp->index; /* destnation port */
typo: destination port
With that fixed:
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH v3 net-next 2/5] phy: micrel: add Microchip KSZ 9477 Switch PHY support
From: Florian Fainelli @ 2017-05-20 0:03 UTC (permalink / raw)
To: Woojung.Huh, andrew, vivien.didelot, sergei.shtylyov
Cc: netdev, davem, UNGLinuxDriver
In-Reply-To: <9235D6609DB808459E95D78E17F2E43D40A7C02D@CHN-SV-EXMX02.mchp-main.com>
On 05/19/2017 03:57 PM, Woojung.Huh@microchip.com wrote:
> From: Woojung Huh <Woojung.Huh@microchip.com>
>
> Adding Microchip 9477 Phy included in KSZ9477 Switch.
>
> Signed-off-by: Woojung Huh <Woojung.Huh@microchip.com>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH v3 net-next 5/5] dsa: add maintainer of Microchip KSZ switches
From: Florian Fainelli @ 2017-05-20 0:03 UTC (permalink / raw)
To: Woojung.Huh, andrew, davem, sergei.shtylyov; +Cc: netdev, UNGLinuxDriver
In-Reply-To: <9235D6609DB808459E95D78E17F2E43D40A7C038@CHN-SV-EXMX02.mchp-main.com>
On 05/19/2017 03:57 PM, Woojung.Huh@microchip.com wrote:
> From: Woojung Huh <Woojung.Huh@microchip.com>
>
> Adding maintainer of Microchip KSZ switches.
>
> Signed-off-by: Woojung Huh <Woojung.Huh@microchip.com>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> MAINTAINERS | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f7d568b..a72b40c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8454,6 +8454,16 @@ F: drivers/media/platform/atmel/atmel-isc.c
> F: drivers/media/platform/atmel/atmel-isc-regs.h
> F: devicetree/bindings/media/atmel-isc.txt
>
> +MICROCHIP KSZ SERIES ETHERNET SWITCH DRIVER
> +M: Woojung Huh <Woojung.Huh@microchip.com>
> +M: Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
> +L: netdev@vger.kernel.org
> +S: Maintained
> +F: net/dsa/tag_ksz.c
> +F: drivers/net/dsa/microchip/*
> +F: include/linux/platform_data/microchip-ksz.h
> +F: Documentation/devicetree/bindings/net/dsa/ksz.txt
> +
> MICROCHIP USB251XB DRIVER
> M: Richard Leitner <richard.leitner@skidata.com>
> L: linux-usb@vger.kernel.org
>
--
Florian
^ permalink raw reply
* Re: [PATCH v3 net-next 4/5] dsa: Add spi support to Microchip KSZ switches
From: Florian Fainelli @ 2017-05-20 0:05 UTC (permalink / raw)
To: Woojung.Huh, andrew, vivien.didelot, sergei.shtylyov
Cc: netdev, davem, UNGLinuxDriver
In-Reply-To: <9235D6609DB808459E95D78E17F2E43D40A7C042@CHN-SV-EXMX02.mchp-main.com>
On 05/19/2017 03:57 PM, Woojung.Huh@microchip.com wrote:
> From: Woojung Huh <Woojung.Huh@microchip.com>
>
> A sample SPI configuration for Microchip KSZ switches.
>
> Signed-off-by: Woojung Huh <Woojung.Huh@microchip.com>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Subject should be something like:
dt-bindings: net: dsa: Add Microchip KSZ switches binding
With that fixed and the nits below:
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> Documentation/devicetree/bindings/net/dsa/ksz.txt | 73 +++++++++++++++++++++++
> 1 file changed, 73 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/dsa/ksz.txt
>
> diff --git a/Documentation/devicetree/bindings/net/dsa/ksz.txt b/Documentation/devicetree/bindings/net/dsa/ksz.txt
> new file mode 100644
> index 0000000..8a13966
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/dsa/ksz.txt
> @@ -0,0 +1,73 @@
> +Microchip KSZ Series Ethernet switches
> +==================================
> +
> +Required properties:
> +
> +- compatible: For external switch chips, compatible string must be exactly one
> + of: "microchip,ksz9477"
> +
> +See Documentation/devicetree/bindings/dsa/dsa.txt for a list of additional
> +required and optional properties.
> +
> +Examples:
> +
> +Ethernet switch connected via SPI to the host, CPU port wired to eth0:
> +
> + eth0: ethernet@10001000 {
> + fixed-link {
> + reg = <7>
There is a missing semicolon, and for a fixed-link, there is no "reg"
property.
> + speed = <1000>;
> + duplex-full;
Actually the correct property is named "full-duplex" (like you put it
for the switch)
> + };
> + };
> +
> + spi1: spi@f8008000 {
> + pinctrl-0 = <&pinctrl_spi_ksz>;
> + cs-gpios = <&pioC 25 0>;
> + id = <1>;
> + status = "okay";
> +
> + ksz9477: ksz9477@0 {
> + compatible = "microchip,ksz9477";
> + reg = <0>;
> +
> + spi-max-frequency = <44000000>;
> + spi-cpha;
> + spi-cpol;
> +
> + status = "okay";
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + port@0 {
> + reg = <0>;
> + label = "lan1";
> + };
> + port@1 {
> + reg = <1>;
> + label = "lan2";
> + };
> + port@2 {
> + reg = <2>;
> + label = "lan3";
> + };
> + port@3 {
> + reg = <3>;
> + label = "lan4";
> + };
> + port@4 {
> + reg = <4>;
> + label = "lan5";
> + };
> + port@5 {
> + reg = <5>;
> + label = "cpu";
> + ethernet = <ð0>;
> + fixed-link {
> + speed = <1000>;
> + full-duplex;
> + };
> + };
> + };
> + };
> + };
>
--
Florian
^ permalink raw reply
* Re: [PATCH v3 net-next 3/5] dsa: add DSA switch driver for Microchip KSZ9477
From: Florian Fainelli @ 2017-05-20 0:10 UTC (permalink / raw)
To: Woojung.Huh, andrew, vivien.didelot, sergei.shtylyov
Cc: netdev, davem, UNGLinuxDriver
In-Reply-To: <9235D6609DB808459E95D78E17F2E43D40A7C023@CHN-SV-EXMX02.mchp-main.com>
On 05/19/2017 03:57 PM, Woojung.Huh@microchip.com wrote:
> From: Woojung Huh <Woojung.Huh@microchip.com>
>
> The KSZ9477 is a fully integrated layer 2, managed, 7 ports GigE switch
> with numerous advanced features. 5 ports incorporate 10/100/1000 Mbps PHYs.
> The other 2 ports have interfaces that can be configured as SGMII, RGMII, MII
> or RMII. Either of these may connect directly to a host processor or
> to an external PHY. The SGMII port may interface to a fiber optic transceiver.
>
> This driver currently supports vlan, fdb, mdb & mirror dsa switch operations.
>
> Signed-off-by: Woojung Huh <Woojung.Huh@microchip.com>
Looks great, thanks Woojung!
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH v2 1/3] bpf: Use 1<<16 as ceiling for immediate alignment in verifier.
From: Alexei Starovoitov @ 2017-05-20 0:20 UTC (permalink / raw)
To: David Miller; +Cc: ecree, daniel, alexei.starovoitov, netdev
In-Reply-To: <20170519.191615.136362788931426782.davem@davemloft.net>
On 5/19/17 4:16 PM, David Miller wrote:
> From: Alexei Starovoitov <ast@fb.com>
> Date: Fri, 19 May 2017 14:37:56 -0700
>
>> On 5/19/17 1:41 PM, David Miller wrote:
>>> From: Edward Cree <ecree@solarflare.com>
>>> Date: Fri, 19 May 2017 18:17:42 +0100
>>>
>>>> One question: is there a way to build the verifier as userland code
>>>> (or at least as a module), or will I have to reboot every time I
>>>> want to test a change?
>>>
>>> There currently is no such machanism, you will have to reboot every
>>> time.
>>>
>>> I have considered working on making the code buildable outside of the
>>> kernel. It shouldn't be too hard.
>>
>> it's not hard.
>> We did it twice and both times abandoned.
>> First time to have 'user space verifier' to check programs before
>> loading and second time for fuzzing via llvm.
>> Abandoned since it diverges very quickly from kernel.
>>
>
> Well, my idea was the create an environment in which kernel verifier.c
> could be built as-is.
>
> Maybe there would be some small compromises in verifier.c such as an
> ifdef test or two, but that should be it.
that's exactly what we did the first time. Added few ifdef to verifier.c
Second time we went even further by compiling kernel/bpf/verifier.c
as-is and linking everything magically via user space hooks
all the way that test_verifier.c runs as-is but calling
bpf_check() function that was compiled for user space via clang.
That code is here:
https://github.com/iovisor/bpf-fuzzer
It's definitely possible to refresh it and make it work again.
My point that unless we put such 'lets build verifier.c for user space'
as part of tools/testing/selftests/ or something, such project is
destined to bit rot.
^ permalink raw reply
* [PATCH] i40e: Fix incorrect pf->flags
From: Tushar Dave @ 2017-05-20 1:01 UTC (permalink / raw)
To: jeffrey.t.kirsher, intel-wired-lan, netdev
Fix bug introduced by 'commit 47994c119a36e ("i40e: remove
hw_disabled_flags in favor of using separate flag bits")' that
mistakenly wipes out pf->flags.
Signed-off-by: Tushar Dave <tushar.n.dave@oracle.com>
---
drivers/net/ethernet/intel/i40e/i40e_main.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index d5c9c9e..6b98d34 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -8821,9 +8821,9 @@ static int i40e_sw_init(struct i40e_pf *pf)
(pf->hw.aq.api_min_ver > 4))) {
/* Supported in FW API version higher than 1.4 */
pf->flags |= I40E_FLAG_GENEVE_OFFLOAD_CAPABLE;
- pf->flags = I40E_FLAG_HW_ATR_EVICT_CAPABLE;
+ pf->flags |= I40E_FLAG_HW_ATR_EVICT_CAPABLE;
} else {
- pf->flags = I40E_FLAG_HW_ATR_EVICT_CAPABLE;
+ pf->flags |= I40E_FLAG_HW_ATR_EVICT_CAPABLE;
}
pf->eeprom_version = 0xDEAD;
--
1.9.1
^ permalink raw reply related
* Re: [PATCH net-next] geneve: always fill CSUM6_RX configuration
From: Pravin Shelar @ 2017-05-20 1:57 UTC (permalink / raw)
To: Eric Garver; +Cc: Linux Kernel Network Developers
In-Reply-To: <20170518195956.12686-1-e@erig.me>
On Thu, May 18, 2017 at 12:59 PM, Eric Garver <e@erig.me> wrote:
> CSMU6_RX is relevant for collect_metadata as well. As such leave it
> outside of the dev's IPv4/IPv6 checks.
>
Can you explain it bit? is this flag used with ipv4 tunnels?
> Fixes: 9b4437a5b870 ("geneve: Unify LWT and netdev handling.")
> Signed-off-by: Eric Garver <e@erig.me>
> ---
> drivers/net/geneve.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
> index dec5d563ab19..f557d1dc3f9b 100644
> --- a/drivers/net/geneve.c
> +++ b/drivers/net/geneve.c
> @@ -1311,13 +1311,13 @@ static int geneve_fill_info(struct sk_buff *skb, const struct net_device *dev)
> if (nla_put_u8(skb, IFLA_GENEVE_UDP_ZERO_CSUM6_TX,
> !(info->key.tun_flags & TUNNEL_CSUM)))
> goto nla_put_failure;
> -
> - if (nla_put_u8(skb, IFLA_GENEVE_UDP_ZERO_CSUM6_RX,
> - !geneve->use_udp6_rx_checksums))
> - goto nla_put_failure;
> #endif
> }
>
> + if (nla_put_u8(skb, IFLA_GENEVE_UDP_ZERO_CSUM6_RX,
> + !geneve->use_udp6_rx_checksums))
> + goto nla_put_failure;
> +
> if (nla_put_u8(skb, IFLA_GENEVE_TTL, info->key.ttl) ||
> nla_put_u8(skb, IFLA_GENEVE_TOS, info->key.tos) ||
> nla_put_be32(skb, IFLA_GENEVE_LABEL, info->key.label))
> --
> 2.12.0
>
^ permalink raw reply
* RE: [PATCH v3 net-next 1/5] dsa: add support for Microchip KSZ tail tagging
From: Woojung.Huh @ 2017-05-20 2:12 UTC (permalink / raw)
To: f.fainelli, andrew, vivien.didelot, sergei.shtylyov
Cc: netdev, davem, UNGLinuxDriver
In-Reply-To: <11dd6b8d-855c-0c97-a10b-79b2b7865eeb@gmail.com>
>> + if (padlen) {
>> + u8 *pad = skb_put(nskb, padlen);
>> +
>> + memset(pad, 0, padlen);
>> + }
>
>Can you use skb_put_padto() here instead of open coding this?
>
>> +
>> + tag = skb_put(nskb, KSZ_INGRESS_TAG_LEN);
>> + tag[0] = 0;
>> + tag[1] = 1 << p->dp->index; /* destnation port */
>
>typo: destination port
>
>With that fixed:
>
>Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
HI Florian,
Thanks for prompt reviews. Will submit another version.
- Woojung
^ permalink raw reply
* Re: [RFC net-next PATCH 4/5] net: new XDP feature for reading HW rxhash from drivers
From: Alexei Starovoitov @ 2017-05-20 3:07 UTC (permalink / raw)
To: Jesper Dangaard Brouer; +Cc: Daniel Borkmann, John Fastabend, netdev
In-Reply-To: <149512210827.14733.13997041998775151648.stgit@firesoul>
On Thu, May 18, 2017 at 05:41:48PM +0200, Jesper Dangaard Brouer wrote:
>
> +/* XDP rxhash have an associated type, which is related to the RSS
> + * (Receive Side Scaling) standard, but NIC HW have different mapping
> + * and support. Thus, create mapping that is interesting for XDP. XDP
> + * would primarly want insight into L3 and L4 protocol info.
> + *
> + * TODO: Likely need to get extended with "L3_IPV6_EX" due RSS standard
> + *
> + * The HASH_TYPE will be returned from bpf helper as the top 32-bit of
> + * the 64-bit rxhash (internally type stored in xdp_buff->flags).
> + */
> +#define XDP_HASH(x) ((x) & ((1ULL << 32)-1))
> +#define XDP_HASH_TYPE(x) ((x) >> 32)
> +
> +#define XDP_HASH_TYPE_L3_SHIFT 0
> +#define XDP_HASH_TYPE_L3_BITS 3
> +#define XDP_HASH_TYPE_L3_MASK ((1ULL << XDP_HASH_TYPE_L3_BITS)-1)
> +#define XDP_HASH_TYPE_L3(x) ((x) & XDP_HASH_TYPE_L3_MASK)
> +enum {
> + XDP_HASH_TYPE_L3_IPV4 = 1,
> + XDP_HASH_TYPE_L3_IPV6,
> +};
> +
> +#define XDP_HASH_TYPE_L4_SHIFT XDP_HASH_TYPE_L3_BITS
> +#define XDP_HASH_TYPE_L4_BITS 5
> +#define XDP_HASH_TYPE_L4_MASK \
> + (((1ULL << XDP_HASH_TYPE_L4_BITS)-1) << XDP_HASH_TYPE_L4_SHIFT)
> +#define XDP_HASH_TYPE_L4(x) ((x) & XDP_HASH_TYPE_L4_MASK)
> +enum {
> + _XDP_HASH_TYPE_L4_TCP = 1,
> + _XDP_HASH_TYPE_L4_UDP,
> +};
> +#define XDP_HASH_TYPE_L4_TCP (_XDP_HASH_TYPE_L4_TCP << XDP_HASH_TYPE_L4_SHIFT)
> +#define XDP_HASH_TYPE_L4_UDP (_XDP_HASH_TYPE_L4_UDP << XDP_HASH_TYPE_L4_SHIFT)
imo this is dangerous territory.
As far as I can see this information doesn't exist in the current drivers at all
and you're enabling it in the patch 5 via fancy:
+ u32 ht = (mlx5_htype_l4_to_xdp[((cht & CQE_RSS_HTYPE_L4) >> 6)] | \
+ mlx5_htype_l3_to_xdp[((cht & CQE_RSS_HTYPE_IP) >> 2)]);
It's pretty cool that you've discovered this hidden mlx5 feature
Did you find it in some hw spec ?
And it looks useful to me, but
1. i'm worried that we'd be relying on something that mellanox didn't
implement in their drivers before. Was it tested and guarnteed to exist
in the future revisions of firmware? Is it cx4 or cx4-lx or cx5 feature?
2. but the main concern that it is mellanox only feature. At least I cannot
see anything like this in broadcom and intel nics
In the very beginning we discussed that XDP programs should be as generic as
possible and HW independent while at the same time we want to expose HW
specific features to XDP programs.
So I'm totally fine to expose this fancy hw hash and ipv4 vs v6 and tcp vs udp
flags to xdp programs somehow, but I'm completely against making it into uapi.
How about exposing 'struct mlx5_cqe64 *' to XDP programs as-is?
We can make sure that XDP program does read only access into it and
it will see cqe->rss_hash_result, cqe->rss_hash_type and everything else
in there, but this will not be uapi and it will be pretty obvious
to program authors that their programs are vendor specific.
'not uapi' here means that mellanox is free to change their HW descriptor
and its contents as they wish.
Also no copies and bit conversions will be necessary, so the cost will
be zero to programs that don't use it and we wouldn't need to change
verifier to discover access to this stuff.
Note that bpf programs already have access to all in-kernel data structures
on the tracing side, so this is nothing new and tracing program authors
got used to structures changing from kernel to kernel.
XDP program authors can do the same for vendor specific bits while we
keep core XDP uapi generic across all nics.
^ permalink raw reply
* Re: [RFC net-next PATCH 4/5] net: new XDP feature for reading HW rxhash from drivers
From: Jakub Kicinski @ 2017-05-20 3:21 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Jesper Dangaard Brouer, Daniel Borkmann, John Fastabend, netdev
In-Reply-To: <20170520030750.c2b55oqdms5n764c@ast-mbp>
On Fri, 19 May 2017 20:07:52 -0700, Alexei Starovoitov wrote:
> How about exposing 'struct mlx5_cqe64 *' to XDP programs as-is?
> We can make sure that XDP program does read only access into it and
> it will see cqe->rss_hash_result, cqe->rss_hash_type and everything else
> in there, but this will not be uapi and it will be pretty obvious
> to program authors that their programs are vendor specific.
> 'not uapi' here means that mellanox is free to change their HW descriptor
> and its contents as they wish.
Hm.. Would that mean we have to teach the verifier about all possible
drivers and their metadata structures (i.e. sizes thereof). And add an
UAPI enum of known drivers?
Other idea I floated in early days was to standardize the fields but
let the driver "JIT" the accesses to look at the right offset of the
right structure. Admittedly that would be a lot more work.
^ permalink raw reply
* Re: [RFC net-next PATCH 4/5] net: new XDP feature for reading HW rxhash from drivers
From: Alexei Starovoitov @ 2017-05-20 3:34 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Jesper Dangaard Brouer, Daniel Borkmann, John Fastabend, netdev
In-Reply-To: <20170519202147.3c6bda7f@cakuba.netronome.com>
On Fri, May 19, 2017 at 08:21:47PM -0700, Jakub Kicinski wrote:
> On Fri, 19 May 2017 20:07:52 -0700, Alexei Starovoitov wrote:
> > How about exposing 'struct mlx5_cqe64 *' to XDP programs as-is?
> > We can make sure that XDP program does read only access into it and
> > it will see cqe->rss_hash_result, cqe->rss_hash_type and everything else
> > in there, but this will not be uapi and it will be pretty obvious
> > to program authors that their programs are vendor specific.
> > 'not uapi' here means that mellanox is free to change their HW descriptor
> > and its contents as they wish.
>
> Hm.. Would that mean we have to teach the verifier about all possible
> drivers and their metadata structures (i.e. sizes thereof). And add an
> UAPI enum of known drivers?
why? no uapi other than a pointer to this hw rx descriptor.
Different sizeof(hw_rx_descriptor) is not a problem.
We deal with it already in tracing. All tracepoints have different
sizeof(*ctx), yet the safety is preserved.
> Other idea I floated in early days was to standardize the fields but
> let the driver "JIT" the accesses to look at the right offset of the
> right structure. Admittedly that would be a lot more work.
'standardize the fields' sounds nice, but failed here already.
As far as I can see the meaning of packet 'hash' is quite different
across the drivers and 'hash' is just a beginning.
I hope we can standardize on 'csum' field and make it checksum_complete,
but so far out of 10+G nics only mlx5 and nfp do it in hw.
We need it at least for mlx4, but it can only fake it via expensive math.
^ permalink raw reply
* Re: [RFC net-next PATCH 4/5] net: new XDP feature for reading HW rxhash from drivers
From: Jakub Kicinski @ 2017-05-20 4:13 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: Jesper Dangaard Brouer, Daniel Borkmann, netdev
In-Reply-To: <20170520033358.hjz7ocy2fupwplym@ast-mbp>
On Fri, 19 May 2017 20:34:00 -0700, Alexei Starovoitov wrote:
> On Fri, May 19, 2017 at 08:21:47PM -0700, Jakub Kicinski wrote:
> > On Fri, 19 May 2017 20:07:52 -0700, Alexei Starovoitov wrote:
> > > How about exposing 'struct mlx5_cqe64 *' to XDP programs as-is?
> > > We can make sure that XDP program does read only access into it and
> > > it will see cqe->rss_hash_result, cqe->rss_hash_type and everything else
> > > in there, but this will not be uapi and it will be pretty obvious
> > > to program authors that their programs are vendor specific.
> > > 'not uapi' here means that mellanox is free to change their HW descriptor
> > > and its contents as they wish.
> >
> > Hm.. Would that mean we have to teach the verifier about all possible
> > drivers and their metadata structures (i.e. sizes thereof). And add an
> > UAPI enum of known drivers?
>
> why? no uapi other than a pointer to this hw rx descriptor.
> Different sizeof(hw_rx_descriptor) is not a problem.
> We deal with it already in tracing. All tracepoints have different
> sizeof(*ctx), yet the safety is preserved.
Ack, quick read of tracing code reveals this indeed should work.
^ permalink raw reply
* [net-next] net: ipv6: fix code style error and warning of ndisc.c
From: yuan linyu @ 2017-05-20 4:16 UTC (permalink / raw)
To: netdev; +Cc: David S . Miller, yuan linyu
From: yuan linyu <Linyu.Yuan@alcatel-sbell.com.cn>
Signed-off-by: yuan linyu <Linyu.Yuan@alcatel-sbell.com.cn>
---
net/ipv6/ndisc.c | 300 ++++++++++++++++++++++++++++---------------------------
1 file changed, 155 insertions(+), 145 deletions(-)
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index d310dc4..5a3dfaa 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -12,8 +12,7 @@
* 2 of the License, or (at your option) any later version.
*/
-/*
- * Changes:
+/* Changes:
*
* Alexey I. Froloff : RFC6106 (DNSSL) support
* Pierre Ynard : export userland ND options
@@ -99,7 +98,6 @@ static const struct neigh_ops ndisc_hh_ops = {
.connected_output = neigh_resolve_output,
};
-
static const struct neigh_ops ndisc_direct_ops = {
.family = AF_INET6,
.output = neigh_direct_output,
@@ -147,13 +145,13 @@ void __ndisc_fill_addr_option(struct sk_buff *skb, int type, void *data,
u8 *opt = skb_put(skb, space);
opt[0] = type;
- opt[1] = space>>3;
+ opt[1] = space >> 3;
memset(opt + 2, 0, pad);
opt += pad;
space -= pad;
- memcpy(opt+2, data, data_len);
+ memcpy(opt + 2, data, data_len);
data_len += 2;
opt += data_len;
space -= data_len;
@@ -182,6 +180,7 @@ static struct nd_opt_hdr *ndisc_next_option(struct nd_opt_hdr *cur,
struct nd_opt_hdr *end)
{
int type;
+
if (!cur || !end || cur >= end)
return NULL;
type = cur->nd_opt_type;
@@ -222,6 +221,7 @@ struct ndisc_options *ndisc_parse_options(const struct net_device *dev,
memset(ndopts, 0, sizeof(*ndopts));
while (opt_len) {
int l;
+
if (opt_len < sizeof(struct nd_opt_hdr))
return NULL;
l = nd_opt->nd_opt_len << 3;
@@ -240,13 +240,15 @@ struct ndisc_options *ndisc_parse_options(const struct net_device *dev,
"%s: duplicated ND6 option found: type=%d\n",
__func__, nd_opt->nd_opt_type);
} else {
- ndopts->nd_opt_array[nd_opt->nd_opt_type] = nd_opt;
+ ndopts->nd_opt_array[nd_opt->nd_opt_type] =
+ nd_opt;
}
break;
case ND_OPT_PREFIX_INFO:
ndopts->nd_opts_pi_end = nd_opt;
if (!ndopts->nd_opt_array[nd_opt->nd_opt_type])
- ndopts->nd_opt_array[nd_opt->nd_opt_type] = nd_opt;
+ ndopts->nd_opt_array[nd_opt->nd_opt_type] =
+ nd_opt;
break;
#ifdef CONFIG_IPV6_ROUTE_INFO
case ND_OPT_ROUTE_INFO:
@@ -261,8 +263,7 @@ struct ndisc_options *ndisc_parse_options(const struct net_device *dev,
if (!ndopts->nd_useropts)
ndopts->nd_useropts = nd_opt;
} else {
- /*
- * Unknown options must be silently ignored,
+ /* Unknown options must be silently ignored,
* to accommodate future extension to the
* protocol.
*/
@@ -280,7 +281,8 @@ struct ndisc_options *ndisc_parse_options(const struct net_device *dev,
return ndopts;
}
-int ndisc_mc_map(const struct in6_addr *addr, char *buf, struct net_device *dev, int dir)
+int ndisc_mc_map(const struct in6_addr *addr, char *buf,
+ struct net_device *dev, int dir)
{
switch (dev->type) {
case ARPHRD_ETHER:
@@ -327,9 +329,8 @@ static int ndisc_constructor(struct neighbour *neigh)
bool is_multicast = ipv6_addr_is_multicast(addr);
in6_dev = in6_dev_get(dev);
- if (!in6_dev) {
+ if (!in6_dev)
return -EINVAL;
- }
parms = in6_dev->nd_parms;
__neigh_parms_put(neigh->parms);
@@ -344,12 +345,12 @@ static int ndisc_constructor(struct neighbour *neigh)
if (is_multicast) {
neigh->nud_state = NUD_NOARP;
ndisc_mc_map(addr, neigh->ha, dev, 1);
- } else if (dev->flags&(IFF_NOARP|IFF_LOOPBACK)) {
+ } else if (dev->flags & (IFF_NOARP | IFF_LOOPBACK)) {
neigh->nud_state = NUD_NOARP;
memcpy(neigh->ha, dev->dev_addr, dev->addr_len);
- if (dev->flags&IFF_LOOPBACK)
+ if (dev->flags & IFF_LOOPBACK)
neigh->type = RTN_LOCAL;
- } else if (dev->flags&IFF_POINTOPOINT) {
+ } else if (dev->flags & IFF_POINTOPOINT) {
neigh->nud_state = NUD_NOARP;
memcpy(neigh->ha, dev->broadcast, dev->addr_len);
}
@@ -357,7 +358,7 @@ static int ndisc_constructor(struct neighbour *neigh)
neigh->ops = &ndisc_hh_ops;
else
neigh->ops = &ndisc_generic_ops;
- if (neigh->nud_state&NUD_VALID)
+ if (neigh->nud_state & NUD_VALID)
neigh->output = neigh->ops->connected_output;
else
neigh->output = neigh->ops->output;
@@ -512,7 +513,8 @@ void ndisc_send_na(struct net_device *dev, const struct in6_addr *daddr,
in6_ifa_put(ifp);
} else {
if (ipv6_dev_get_saddr(dev_net(dev), dev, daddr,
- inet6_sk(dev_net(dev)->ipv6.ndisc_sk)->srcprefs,
+ inet6_sk(dev_net(dev)->ipv6.ndisc_sk)->
+ srcprefs,
&tmpaddr))
return;
src_addr = &tmpaddr;
@@ -580,7 +582,7 @@ void ndisc_send_ns(struct net_device *dev, const struct in6_addr *solicit,
if (!saddr) {
if (ipv6_get_lladdr(dev, &addr_buf,
- (IFA_F_TENTATIVE|IFA_F_OPTIMISTIC)))
+ (IFA_F_TENTATIVE | IFA_F_OPTIMISTIC)))
return;
saddr = &addr_buf;
}
@@ -629,8 +631,7 @@ void ndisc_send_rs(struct net_device *dev, const struct in6_addr *saddr,
int optlen = 0;
#ifdef CONFIG_IPV6_OPTIMISTIC_DAD
- /*
- * According to section 2.2 of RFC 4429, we must not
+ /* According to section 2.2 of RFC 4429, we must not
* send router solicitations with a sllao from
* optimistic addresses, but we may send the solicitation
* if we don't include the sllao. So here we check
@@ -641,9 +642,8 @@ void ndisc_send_rs(struct net_device *dev, const struct in6_addr *saddr,
struct inet6_ifaddr *ifp = ipv6_get_ifaddr(dev_net(dev), saddr,
dev, 1);
if (ifp) {
- if (ifp->flags & IFA_F_OPTIMISTIC) {
+ if (ifp->flags & IFA_F_OPTIMISTIC)
send_sllao = 0;
- }
in6_ifa_put(ifp);
} else {
send_sllao = 0;
@@ -672,11 +672,9 @@ void ndisc_send_rs(struct net_device *dev, const struct in6_addr *saddr,
ndisc_send_skb(skb, daddr, saddr);
}
-
static void ndisc_error_report(struct neighbour *neigh, struct sk_buff *skb)
{
- /*
- * "The sender MUST return an ICMP
+ /* "The sender MUST return an ICMP
* destination unreachable"
*/
dst_link_failure(skb);
@@ -695,7 +693,7 @@ static void ndisc_solicit(struct neighbour *neigh, struct sk_buff *skb)
if (skb && ipv6_chk_addr_and_flags(dev_net(dev), &ipv6_hdr(skb)->saddr,
dev, 1,
- IFA_F_TENTATIVE|IFA_F_OPTIMISTIC))
+ IFA_F_TENTATIVE | IFA_F_OPTIMISTIC))
saddr = &ipv6_hdr(skb)->saddr;
probes -= NEIGH_VAR(neigh->parms, UCAST_PROBES);
if (probes < 0) {
@@ -705,11 +703,14 @@ static void ndisc_solicit(struct neighbour *neigh, struct sk_buff *skb)
__func__, target);
}
ndisc_send_ns(dev, target, target, saddr, 0);
- } else if ((probes -= NEIGH_VAR(neigh->parms, APP_PROBES)) < 0) {
- neigh_app_ns(neigh);
} else {
- addrconf_addr_solict_mult(target, &mcaddr);
- ndisc_send_ns(dev, target, &mcaddr, saddr, 0);
+ probes -= NEIGH_VAR(neigh->parms, APP_PROBES);
+ if (probes < 0) {
+ neigh_app_ns(neigh);
+ } else {
+ addrconf_addr_solict_mult(target, &mcaddr);
+ ndisc_send_ns(dev, target, &mcaddr, saddr, 0);
+ }
}
}
@@ -765,8 +766,7 @@ static void ndisc_recv_ns(struct sk_buff *skb)
return;
}
- /*
- * RFC2461 7.1.1:
+ /* RFC2461 7.1.1:
* DAD has to be destined for solicited node multicast address.
*/
if (dad && !ipv6_addr_is_solict_mult(daddr)) {
@@ -806,7 +806,7 @@ static void ndisc_recv_ns(struct sk_buff *skb)
ifp = ipv6_get_ifaddr(dev_net(dev), &msg->target, dev, 1);
if (ifp) {
have_ifp:
- if (ifp->flags & (IFA_F_TENTATIVE|IFA_F_OPTIMISTIC)) {
+ if (ifp->flags & (IFA_F_TENTATIVE | IFA_F_OPTIMISTIC)) {
if (dad) {
if (nonce != 0 && ifp->dad_nonce == nonce) {
u8 *np = (u8 *)&nonce;
@@ -817,23 +817,21 @@ static void ndisc_recv_ns(struct sk_buff *skb)
&ifp->addr, np);
goto out;
}
- /*
- * We are colliding with another node
+ /* We are colliding with another node
* who is doing DAD
* so fail our DAD process
*/
addrconf_dad_failure(ifp);
return;
- } else {
- /*
- * This is not a dad solicitation.
- * If we are an optimistic node,
- * we should respond.
- * Otherwise, we should ignore it.
- */
- if (!(ifp->flags & IFA_F_OPTIMISTIC))
- goto out;
}
+
+ /* This is not a dad solicitation.
+ * If we are an optimistic node,
+ * we should respond.
+ * Otherwise, we should ignore it.
+ */
+ if (!(ifp->flags & IFA_F_OPTIMISTIC))
+ goto out;
}
idev = ifp->idev;
@@ -846,7 +844,8 @@ static void ndisc_recv_ns(struct sk_buff *skb)
mdev = netdev_master_upper_dev_get_rcu(dev);
if (mdev) {
- ifp = ipv6_get_ifaddr(net, &msg->target, mdev, 1);
+ ifp = ipv6_get_ifaddr(net, &msg->target,
+ mdev, 1);
if (ifp)
goto have_ifp;
}
@@ -858,28 +857,31 @@ static void ndisc_recv_ns(struct sk_buff *skb)
return;
}
+ is_router = pndisc_is_router(&msg->target, dev);
if (ipv6_chk_acast_addr(net, dev, &msg->target) ||
(idev->cnf.forwarding &&
- (net->ipv6.devconf_all->proxy_ndp || idev->cnf.proxy_ndp) &&
- (is_router = pndisc_is_router(&msg->target, dev)) >= 0)) {
+ (net->ipv6.devconf_all->proxy_ndp ||
+ idev->cnf.proxy_ndp) && is_router >= 0)) {
if (!(NEIGH_CB(skb)->flags & LOCALLY_ENQUEUED) &&
skb->pkt_type != PACKET_HOST &&
inc &&
NEIGH_VAR(idev->nd_parms, PROXY_DELAY) != 0) {
- /*
- * for anycast or proxy,
+ /* for anycast or proxy,
* sender should delay its response
* by a random time between 0 and
* MAX_ANYCAST_DELAY_TIME seconds.
* (RFC2461) -- yoshfuji
*/
struct sk_buff *n = skb_clone(skb, GFP_ATOMIC);
+
if (n)
- pneigh_enqueue(&nd_tbl, idev->nd_parms, n);
+ pneigh_enqueue(&nd_tbl,
+ idev->nd_parms, n);
goto out;
}
- } else
+ } else {
goto out;
+ }
}
if (is_router < 0)
@@ -887,7 +889,7 @@ static void ndisc_recv_ns(struct sk_buff *skb)
if (dad) {
ndisc_send_na(dev, &in6addr_linklocal_allnodes, &msg->target,
- !!is_router, false, (ifp != NULL), true);
+ !!is_router, false, ifp, true);
goto out;
}
@@ -896,20 +898,19 @@ static void ndisc_recv_ns(struct sk_buff *skb)
else
NEIGH_CACHE_STAT_INC(&nd_tbl, rcv_probes_ucast);
- /*
- * update / create cache entry
+ /* update / create cache entry
* for the source address
*/
neigh = __neigh_lookup(&nd_tbl, saddr, dev,
!inc || lladdr || !dev->addr_len);
if (neigh)
ndisc_update(dev, neigh, lladdr, NUD_STALE,
- NEIGH_UPDATE_F_WEAK_OVERRIDE|
+ NEIGH_UPDATE_F_WEAK_OVERRIDE |
NEIGH_UPDATE_F_OVERRIDE,
NDISC_NEIGHBOUR_SOLICITATION, &ndopts);
if (neigh || !dev->header_ops) {
ndisc_send_na(dev, saddr, &msg->target, !!is_router,
- true, (ifp != NULL && inc), inc);
+ true, (ifp && inc), inc);
if (neigh)
neigh_release(neigh);
}
@@ -973,19 +974,19 @@ static void ndisc_recv_na(struct sk_buff *skb)
}
ifp = ipv6_get_ifaddr(dev_net(dev), &msg->target, dev, 1);
if (ifp) {
- if (skb->pkt_type != PACKET_LOOPBACK
- && (ifp->flags & IFA_F_TENTATIVE)) {
- addrconf_dad_failure(ifp);
- return;
+ if (skb->pkt_type != PACKET_LOOPBACK &&
+ (ifp->flags & IFA_F_TENTATIVE)) {
+ addrconf_dad_failure(ifp);
+ return;
}
/* What should we make now? The advertisement
- is invalid, but ndisc specs say nothing
- about it. It could be misconfiguration, or
- an smart proxy agent tries to help us :-)
-
- We should not print the error if NA has been
- received from loopback - it is just our own
- unsolicited advertisement.
+ * is invalid, but ndisc specs say nothing
+ * about it. It could be misconfiguration, or
+ * an smart proxy agent tries to help us :-)
+ *
+ * We should not print the error if NA has been
+ * received from loopback - it is just our own
+ * unsolicited advertisement.
*/
if (skb->pkt_type != PACKET_LOOPBACK)
ND_PRINTK(1, warn,
@@ -1003,30 +1004,31 @@ static void ndisc_recv_na(struct sk_buff *skb)
if (neigh->nud_state & NUD_FAILED)
goto out;
- /*
- * Don't update the neighbor cache entry on a proxy NA from
+ /* Don't update the neighbor cache entry on a proxy NA from
* ourselves because either the proxied node is off link or it
* has already sent a NA to us.
*/
if (lladdr && !memcmp(lladdr, dev->dev_addr, dev->addr_len) &&
- net->ipv6.devconf_all->forwarding && net->ipv6.devconf_all->proxy_ndp &&
+ net->ipv6.devconf_all->forwarding &&
+ net->ipv6.devconf_all->proxy_ndp &&
pneigh_lookup(&nd_tbl, net, &msg->target, dev, 0)) {
/* XXX: idev->cnf.proxy_ndp */
goto out;
}
ndisc_update(dev, neigh, lladdr,
- msg->icmph.icmp6_solicited ? NUD_REACHABLE : NUD_STALE,
- NEIGH_UPDATE_F_WEAK_OVERRIDE|
- (msg->icmph.icmp6_override ? NEIGH_UPDATE_F_OVERRIDE : 0)|
- NEIGH_UPDATE_F_OVERRIDE_ISROUTER|
- (msg->icmph.icmp6_router ? NEIGH_UPDATE_F_ISROUTER : 0),
+ msg->icmph.icmp6_solicited ?
+ NUD_REACHABLE : NUD_STALE,
+ NEIGH_UPDATE_F_WEAK_OVERRIDE |
+ (msg->icmph.icmp6_override ?
+ NEIGH_UPDATE_F_OVERRIDE : 0) |
+ NEIGH_UPDATE_F_OVERRIDE_ISROUTER |
+ (msg->icmph.icmp6_router ?
+ NEIGH_UPDATE_F_ISROUTER : 0),
NDISC_NEIGHBOUR_ADVERTISEMENT, &ndopts);
if ((old_flags & ~neigh->flags) & NTF_ROUTER) {
- /*
- * Change: router to host
- */
+ /* Change: router to host */
rt6_clean_tohost(dev_net(dev), saddr);
}
@@ -1058,8 +1060,7 @@ static void ndisc_recv_rs(struct sk_buff *skb)
if (!idev->cnf.forwarding)
goto out;
- /*
- * Don't update NCE if src = ::;
+ /* Don't update NCE if src = ::;
* this implies that the source node has no ip address assigned yet.
*/
if (ipv6_addr_any(saddr))
@@ -1081,8 +1082,8 @@ static void ndisc_recv_rs(struct sk_buff *skb)
neigh = __neigh_lookup(&nd_tbl, saddr, skb->dev, 1);
if (neigh) {
ndisc_update(skb->dev, neigh, lladdr, NUD_STALE,
- NEIGH_UPDATE_F_WEAK_OVERRIDE|
- NEIGH_UPDATE_F_OVERRIDE|
+ NEIGH_UPDATE_F_WEAK_OVERRIDE |
+ NEIGH_UPDATE_F_OVERRIDE |
NEIGH_UPDATE_F_OVERRIDE_ISROUTER,
NDISC_ROUTER_SOLICITATION, &ndopts);
neigh_release(neigh);
@@ -1110,9 +1111,8 @@ static void ndisc_ra_useropt(struct sk_buff *ra, struct nd_opt_hdr *opt)
}
nlh = nlmsg_put(skb, 0, 0, RTM_NEWNDUSEROPT, base_size, 0);
- if (!nlh) {
+ if (!nlh)
goto nla_put_failure;
- }
ndmsg = nlmsg_data(nlh);
ndmsg->nduseropt_family = AF_INET6;
@@ -1174,9 +1174,7 @@ static void ndisc_router_discovery(struct sk_buff *skb)
}
#endif
- /*
- * set the RA_RECV flag in the interface
- */
+ /* set the RA_RECV flag in the interface */
in6_dev = __in6_dev_get(skb->dev);
if (!in6_dev) {
@@ -1208,15 +1206,13 @@ static void ndisc_router_discovery(struct sk_buff *skb)
#endif
if (in6_dev->if_flags & IF_RS_SENT) {
- /*
- * flag that an RA was received after an RS was sent
+ /* flag that an RA was received after an RS was sent
* out on this interface.
*/
in6_dev->if_flags |= IF_RA_RCVD;
}
- /*
- * Remember the managed/otherconf flags from most recently
+ /* Remember the managed/otherconf flags from most recently
* received RA message (RFC 2462) -- yoshfuji
*/
old_if_flags = in6_dev->if_flags;
@@ -1299,77 +1295,79 @@ static void ndisc_router_discovery(struct sk_buff *skb)
}
neigh->flags |= NTF_ROUTER;
} else if (rt) {
- rt->rt6i_flags = (rt->rt6i_flags & ~RTF_PREF_MASK) | RTF_PREF(pref);
+ rt->rt6i_flags =
+ (rt->rt6i_flags & ~RTF_PREF_MASK) | RTF_PREF(pref);
}
if (rt)
rt6_set_expires(rt, jiffies + (HZ * lifetime));
if (in6_dev->cnf.accept_ra_min_hop_limit < 256 &&
ra_msg->icmph.icmp6_hop_limit) {
- if (in6_dev->cnf.accept_ra_min_hop_limit <= ra_msg->icmph.icmp6_hop_limit) {
+ if (in6_dev->cnf.accept_ra_min_hop_limit <=
+ ra_msg->icmph.icmp6_hop_limit) {
in6_dev->cnf.hop_limit = ra_msg->icmph.icmp6_hop_limit;
if (rt)
dst_metric_set(&rt->dst, RTAX_HOPLIMIT,
ra_msg->icmph.icmp6_hop_limit);
} else {
- ND_PRINTK(2, warn, "RA: Got route advertisement with lower hop_limit than minimum\n");
+ ND_PRINTK(2, warn,
+ "RA: Got route advertisement with lower hop_limit than minimum\n");
}
}
skip_defrtr:
- /*
- * Update Reachable Time and Retrans Timer
- */
+ /* Update Reachable Time and Retrans Timer */
if (in6_dev->nd_parms) {
unsigned long rtime = ntohl(ra_msg->retrans_timer);
- if (rtime && rtime/1000 < MAX_SCHEDULE_TIMEOUT/HZ) {
- rtime = (rtime*HZ)/1000;
- if (rtime < HZ/10)
- rtime = HZ/10;
+ if (rtime && rtime / 1000 < MAX_SCHEDULE_TIMEOUT / HZ) {
+ rtime = (rtime * HZ) / 1000;
+ if (rtime < HZ / 10)
+ rtime = HZ / 10;
NEIGH_VAR_SET(in6_dev->nd_parms, RETRANS_TIME, rtime);
in6_dev->tstamp = jiffies;
send_ifinfo_notify = true;
}
rtime = ntohl(ra_msg->reachable_time);
- if (rtime && rtime/1000 < MAX_SCHEDULE_TIMEOUT/(3*HZ)) {
- rtime = (rtime*HZ)/1000;
+ if (rtime && rtime / 1000 < MAX_SCHEDULE_TIMEOUT / (3 * HZ)) {
+ rtime = (rtime * HZ) / 1000;
- if (rtime < HZ/10)
- rtime = HZ/10;
+ if (rtime < HZ / 10)
+ rtime = HZ / 10;
- if (rtime != NEIGH_VAR(in6_dev->nd_parms, BASE_REACHABLE_TIME)) {
+ if (rtime != NEIGH_VAR(in6_dev->nd_parms,
+ BASE_REACHABLE_TIME)) {
NEIGH_VAR_SET(in6_dev->nd_parms,
BASE_REACHABLE_TIME, rtime);
NEIGH_VAR_SET(in6_dev->nd_parms,
GC_STALETIME, 3 * rtime);
- in6_dev->nd_parms->reachable_time = neigh_rand_reach_time(rtime);
+ in6_dev->nd_parms->reachable_time =
+ neigh_rand_reach_time(rtime);
in6_dev->tstamp = jiffies;
send_ifinfo_notify = true;
}
}
}
- /*
- * Send a notify if RA changed managed/otherconf flags or timer settings
+ /* Send a notify if RA changed managed/otherconf flags
+ * or timer settings
*/
if (send_ifinfo_notify)
inet6_ifinfo_notify(RTM_NEWLINK, in6_dev);
skip_linkparms:
- /*
- * Process options.
- */
+ /* Process options */
if (!neigh)
neigh = __neigh_lookup(&nd_tbl, &ipv6_hdr(skb)->saddr,
skb->dev, 1);
if (neigh) {
u8 *lladdr = NULL;
+
if (ndopts.nd_opts_src_lladdr) {
lladdr = ndisc_opt_addr_data(ndopts.nd_opts_src_lladdr,
skb->dev);
@@ -1380,9 +1378,9 @@ static void ndisc_router_discovery(struct sk_buff *skb)
}
}
ndisc_update(skb->dev, neigh, lladdr, NUD_STALE,
- NEIGH_UPDATE_F_WEAK_OVERRIDE|
- NEIGH_UPDATE_F_OVERRIDE|
- NEIGH_UPDATE_F_OVERRIDE_ISROUTER|
+ NEIGH_UPDATE_F_WEAK_OVERRIDE |
+ NEIGH_UPDATE_F_OVERRIDE |
+ NEIGH_UPDATE_F_OVERRIDE_ISROUTER |
NEIGH_UPDATE_F_ISROUTER,
NDISC_ROUTER_ADVERTISEMENT, &ndopts);
}
@@ -1406,6 +1404,7 @@ static void ndisc_router_discovery(struct sk_buff *skb)
if (in6_dev->cnf.accept_ra_rtr_pref && ndopts.nd_opts_ri) {
struct nd_opt_hdr *p;
+
for (p = ndopts.nd_opts_ri;
p;
p = ndisc_next_option(p, ndopts.nd_opts_ri_end)) {
@@ -1418,9 +1417,11 @@ static void ndisc_router_discovery(struct sk_buff *skb)
if (ri->prefix_len == 0 &&
!in6_dev->cnf.accept_ra_defrtr)
continue;
- if (ri->prefix_len < in6_dev->cnf.accept_ra_rt_info_min_plen)
+ if (ri->prefix_len <
+ in6_dev->cnf.accept_ra_rt_info_min_plen)
continue;
- if (ri->prefix_len > in6_dev->cnf.accept_ra_rt_info_max_plen)
+ if (ri->prefix_len >
+ in6_dev->cnf.accept_ra_rt_info_max_plen)
continue;
rt6_route_rcv(skb->dev, (u8 *)p, (p->nd_opt_len) << 3,
&ipv6_hdr(skb)->saddr);
@@ -1442,12 +1443,13 @@ static void ndisc_router_discovery(struct sk_buff *skb)
if (in6_dev->cnf.accept_ra_pinfo && ndopts.nd_opts_pi) {
struct nd_opt_hdr *p;
+
for (p = ndopts.nd_opts_pi;
p;
p = ndisc_next_option(p, ndopts.nd_opts_pi_end)) {
addrconf_prefix_rcv(skb->dev, (u8 *)p,
(p->nd_opt_len) << 3,
- ndopts.nd_opts_src_lladdr != NULL);
+ ndopts.nd_opts_src_lladdr);
}
}
@@ -1455,7 +1457,7 @@ static void ndisc_router_discovery(struct sk_buff *skb)
__be32 n;
u32 mtu;
- memcpy(&n, ((u8 *)(ndopts.nd_opts_mtu+1))+2, sizeof(mtu));
+ memcpy(&n, ((u8 *)(ndopts.nd_opts_mtu + 1)) + 2, sizeof(mtu));
mtu = ntohl(n);
if (mtu < IPV6_MIN_MTU || mtu > skb->dev->mtu) {
@@ -1472,6 +1474,7 @@ static void ndisc_router_discovery(struct sk_buff *skb)
if (ndopts.nd_useropts) {
struct nd_opt_hdr *p;
+
for (p = ndopts.nd_useropts;
p;
p = ndisc_next_useropt(skb->dev, p,
@@ -1480,9 +1483,9 @@ static void ndisc_router_discovery(struct sk_buff *skb)
}
}
- if (ndopts.nd_opts_tgt_lladdr || ndopts.nd_opts_rh) {
+ if (ndopts.nd_opts_tgt_lladdr || ndopts.nd_opts_rh)
ND_PRINTK(2, warn, "RA: invalid RA options\n");
- }
+
out:
ip6_rt_put(rt);
if (neigh)
@@ -1518,7 +1521,7 @@ static void ndisc_redirect_rcv(struct sk_buff *skb)
if (!ndopts.nd_opts_rh) {
ip6_redirect_no_header(skb, dev_net(skb->dev),
- skb->dev->ifindex, 0);
+ skb->dev->ifindex, 0);
return;
}
@@ -1569,7 +1572,8 @@ void ndisc_send_redirect(struct sk_buff *skb, const struct in6_addr *target)
}
if (!ipv6_addr_equal(&ipv6_hdr(skb)->daddr, target) &&
- ipv6_addr_type(target) != (IPV6_ADDR_UNICAST|IPV6_ADDR_LINKLOCAL)) {
+ ipv6_addr_type(target) !=
+ (IPV6_ADDR_UNICAST | IPV6_ADDR_LINKLOCAL)) {
ND_PRINTK(2, warn,
"Redirect: target address is not link-local unicast\n");
return;
@@ -1587,7 +1591,7 @@ void ndisc_send_redirect(struct sk_buff *skb, const struct in6_addr *target)
if (IS_ERR(dst))
return;
- rt = (struct rt6_info *) dst;
+ rt = (struct rt6_info *)dst;
if (rt->rt6i_flags & RTF_GATEWAY) {
ND_PRINTK(2, warn,
@@ -1595,14 +1599,16 @@ void ndisc_send_redirect(struct sk_buff *skb, const struct in6_addr *target)
goto release;
}
peer = inet_getpeer_v6(net->ipv6.peers, &ipv6_hdr(skb)->saddr, 1);
- ret = inet_peer_xrlim_allow(peer, 1*HZ);
+ ret = inet_peer_xrlim_allow(peer, 1 * HZ);
if (peer)
inet_putpeer(peer);
if (!ret)
goto release;
if (dev->addr_len) {
- struct neighbour *neigh = dst_neigh_lookup(skb_dst(skb), target);
+ struct neighbour *neigh =
+ dst_neigh_lookup(skb_dst(skb), target);
+
if (!neigh) {
ND_PRINTK(2, warn,
"Redirect: no neigh for target address\n");
@@ -1617,14 +1623,16 @@ void ndisc_send_redirect(struct sk_buff *skb, const struct in6_addr *target)
optlen += ndisc_redirect_opt_addr_space(dev, neigh,
ops_data_buf,
&ops_data);
- } else
+ } else {
read_unlock_bh(&neigh->lock);
+ }
neigh_release(neigh);
}
rd_len = min_t(unsigned int,
- IPV6_MIN_MTU - sizeof(struct ipv6hdr) - sizeof(*msg) - optlen,
+ IPV6_MIN_MTU - sizeof(struct ipv6hdr) -
+ sizeof(*msg) - optlen,
skb->len + 8);
rd_len &= ~0x7;
optlen += rd_len;
@@ -1642,16 +1650,12 @@ void ndisc_send_redirect(struct sk_buff *skb, const struct in6_addr *target)
.dest = ipv6_hdr(skb)->daddr,
};
- /*
- * include target_address option
- */
+ /* include target_address option */
if (ha)
ndisc_fill_redirect_addr_option(buff, ha, ops_data);
- /*
- * build redirect option and copy skb over to the new packet.
- */
+ /* build redirect option and copy skb over to the new packet */
if (rd_len)
ndisc_fill_redirect_hdr_option(buff, skb, rd_len);
@@ -1737,7 +1741,8 @@ int ndisc_rcv(struct sk_buff *skb)
return 0;
}
-static int ndisc_netdev_event(struct notifier_block *this, unsigned long event, void *ptr)
+static int ndisc_netdev_event(struct notifier_block *this,
+ unsigned long event, void *ptr)
{
struct net_device *dev = netdev_notifier_info_to_dev(ptr);
struct netdev_notifier_change_info *change_info;
@@ -1787,6 +1792,7 @@ static void ndisc_warn_deprecated_sysctl(struct ctl_table *ctl,
{
static char warncomm[TASK_COMM_LEN];
static int warned;
+
if (strcmp(warncomm, current->comm) && warned < 5) {
strcpy(warncomm, current->comm);
pr_warn("process `%s' is using deprecated sysctl (%s) net.ipv6.neigh.%s.%s - use net.ipv6.neigh.%s.%s_ms instead\n",
@@ -1797,7 +1803,8 @@ static void ndisc_warn_deprecated_sysctl(struct ctl_table *ctl,
}
}
-int ndisc_ifinfo_sysctl_change(struct ctl_table *ctl, int write, void __user *buffer, size_t *lenp, loff_t *ppos)
+int ndisc_ifinfo_sysctl_change(struct ctl_table *ctl, int write,
+ void __user *buffer, size_t *lenp, loff_t *ppos)
{
struct net_device *dev = ctl->extra1;
struct inet6_dev *idev;
@@ -1805,7 +1812,8 @@ int ndisc_ifinfo_sysctl_change(struct ctl_table *ctl, int write, void __user *bu
if ((strcmp(ctl->procname, "retrans_time") == 0) ||
(strcmp(ctl->procname, "base_reachable_time") == 0))
- ndisc_warn_deprecated_sysctl(ctl, "syscall", dev ? dev->name : "default");
+ ndisc_warn_deprecated_sysctl(ctl, "syscall",
+ dev ? dev->name : "default");
if (strcmp(ctl->procname, "retrans_time") == 0)
ret = neigh_proc_dointvec(ctl, write, buffer, lenp, ppos);
@@ -1821,18 +1829,22 @@ int ndisc_ifinfo_sysctl_change(struct ctl_table *ctl, int write, void __user *bu
else
ret = -1;
- if (write && ret == 0 && dev && (idev = in6_dev_get(dev)) != NULL) {
- if (ctl->data == &NEIGH_VAR(idev->nd_parms, BASE_REACHABLE_TIME))
+ if (!write || ret || !dev)
+ return ret;
+
+ idev = in6_dev_get(dev);
+ if (idev) {
+ if (ctl->data ==
+ &NEIGH_VAR(idev->nd_parms, BASE_REACHABLE_TIME))
idev->nd_parms->reachable_time =
- neigh_rand_reach_time(NEIGH_VAR(idev->nd_parms, BASE_REACHABLE_TIME));
+ neigh_rand_reach_time(
+ NEIGH_VAR(idev->nd_parms, BASE_REACHABLE_TIME));
idev->tstamp = jiffies;
inet6_ifinfo_notify(RTM_NEWLINK, idev);
in6_dev_put(idev);
}
return ret;
}
-
-
#endif
static int __net_init ndisc_net_init(struct net *net)
@@ -1877,9 +1889,7 @@ int __init ndisc_init(void)
err = register_pernet_subsys(&ndisc_net_ops);
if (err)
return err;
- /*
- * Initialize the neighbour table
- */
+ /* Initialize the neighbour table */
neigh_table_init(NEIGH_ND_TABLE, &nd_tbl);
#ifdef CONFIG_SYSCTL
--
2.7.4
^ permalink raw reply related
* [PATCH v2 net-next] net: ipv6: fix code style error and warning of ndisc.c
From: yuan linyu @ 2017-05-20 4:20 UTC (permalink / raw)
To: netdev; +Cc: David S . Miller, yuan linyu
From: yuan linyu <Linyu.Yuan@alcatel-sbell.com.cn>
Signed-off-by: yuan linyu <Linyu.Yuan@alcatel-sbell.com.cn>
---
net/ipv6/ndisc.c | 300 ++++++++++++++++++++++++++++---------------------------
1 file changed, 155 insertions(+), 145 deletions(-)
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index d310dc4..5a3dfaa 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -12,8 +12,7 @@
* 2 of the License, or (at your option) any later version.
*/
-/*
- * Changes:
+/* Changes:
*
* Alexey I. Froloff : RFC6106 (DNSSL) support
* Pierre Ynard : export userland ND options
@@ -99,7 +98,6 @@ static const struct neigh_ops ndisc_hh_ops = {
.connected_output = neigh_resolve_output,
};
-
static const struct neigh_ops ndisc_direct_ops = {
.family = AF_INET6,
.output = neigh_direct_output,
@@ -147,13 +145,13 @@ void __ndisc_fill_addr_option(struct sk_buff *skb, int type, void *data,
u8 *opt = skb_put(skb, space);
opt[0] = type;
- opt[1] = space>>3;
+ opt[1] = space >> 3;
memset(opt + 2, 0, pad);
opt += pad;
space -= pad;
- memcpy(opt+2, data, data_len);
+ memcpy(opt + 2, data, data_len);
data_len += 2;
opt += data_len;
space -= data_len;
@@ -182,6 +180,7 @@ static struct nd_opt_hdr *ndisc_next_option(struct nd_opt_hdr *cur,
struct nd_opt_hdr *end)
{
int type;
+
if (!cur || !end || cur >= end)
return NULL;
type = cur->nd_opt_type;
@@ -222,6 +221,7 @@ struct ndisc_options *ndisc_parse_options(const struct net_device *dev,
memset(ndopts, 0, sizeof(*ndopts));
while (opt_len) {
int l;
+
if (opt_len < sizeof(struct nd_opt_hdr))
return NULL;
l = nd_opt->nd_opt_len << 3;
@@ -240,13 +240,15 @@ struct ndisc_options *ndisc_parse_options(const struct net_device *dev,
"%s: duplicated ND6 option found: type=%d\n",
__func__, nd_opt->nd_opt_type);
} else {
- ndopts->nd_opt_array[nd_opt->nd_opt_type] = nd_opt;
+ ndopts->nd_opt_array[nd_opt->nd_opt_type] =
+ nd_opt;
}
break;
case ND_OPT_PREFIX_INFO:
ndopts->nd_opts_pi_end = nd_opt;
if (!ndopts->nd_opt_array[nd_opt->nd_opt_type])
- ndopts->nd_opt_array[nd_opt->nd_opt_type] = nd_opt;
+ ndopts->nd_opt_array[nd_opt->nd_opt_type] =
+ nd_opt;
break;
#ifdef CONFIG_IPV6_ROUTE_INFO
case ND_OPT_ROUTE_INFO:
@@ -261,8 +263,7 @@ struct ndisc_options *ndisc_parse_options(const struct net_device *dev,
if (!ndopts->nd_useropts)
ndopts->nd_useropts = nd_opt;
} else {
- /*
- * Unknown options must be silently ignored,
+ /* Unknown options must be silently ignored,
* to accommodate future extension to the
* protocol.
*/
@@ -280,7 +281,8 @@ struct ndisc_options *ndisc_parse_options(const struct net_device *dev,
return ndopts;
}
-int ndisc_mc_map(const struct in6_addr *addr, char *buf, struct net_device *dev, int dir)
+int ndisc_mc_map(const struct in6_addr *addr, char *buf,
+ struct net_device *dev, int dir)
{
switch (dev->type) {
case ARPHRD_ETHER:
@@ -327,9 +329,8 @@ static int ndisc_constructor(struct neighbour *neigh)
bool is_multicast = ipv6_addr_is_multicast(addr);
in6_dev = in6_dev_get(dev);
- if (!in6_dev) {
+ if (!in6_dev)
return -EINVAL;
- }
parms = in6_dev->nd_parms;
__neigh_parms_put(neigh->parms);
@@ -344,12 +345,12 @@ static int ndisc_constructor(struct neighbour *neigh)
if (is_multicast) {
neigh->nud_state = NUD_NOARP;
ndisc_mc_map(addr, neigh->ha, dev, 1);
- } else if (dev->flags&(IFF_NOARP|IFF_LOOPBACK)) {
+ } else if (dev->flags & (IFF_NOARP | IFF_LOOPBACK)) {
neigh->nud_state = NUD_NOARP;
memcpy(neigh->ha, dev->dev_addr, dev->addr_len);
- if (dev->flags&IFF_LOOPBACK)
+ if (dev->flags & IFF_LOOPBACK)
neigh->type = RTN_LOCAL;
- } else if (dev->flags&IFF_POINTOPOINT) {
+ } else if (dev->flags & IFF_POINTOPOINT) {
neigh->nud_state = NUD_NOARP;
memcpy(neigh->ha, dev->broadcast, dev->addr_len);
}
@@ -357,7 +358,7 @@ static int ndisc_constructor(struct neighbour *neigh)
neigh->ops = &ndisc_hh_ops;
else
neigh->ops = &ndisc_generic_ops;
- if (neigh->nud_state&NUD_VALID)
+ if (neigh->nud_state & NUD_VALID)
neigh->output = neigh->ops->connected_output;
else
neigh->output = neigh->ops->output;
@@ -512,7 +513,8 @@ void ndisc_send_na(struct net_device *dev, const struct in6_addr *daddr,
in6_ifa_put(ifp);
} else {
if (ipv6_dev_get_saddr(dev_net(dev), dev, daddr,
- inet6_sk(dev_net(dev)->ipv6.ndisc_sk)->srcprefs,
+ inet6_sk(dev_net(dev)->ipv6.ndisc_sk)->
+ srcprefs,
&tmpaddr))
return;
src_addr = &tmpaddr;
@@ -580,7 +582,7 @@ void ndisc_send_ns(struct net_device *dev, const struct in6_addr *solicit,
if (!saddr) {
if (ipv6_get_lladdr(dev, &addr_buf,
- (IFA_F_TENTATIVE|IFA_F_OPTIMISTIC)))
+ (IFA_F_TENTATIVE | IFA_F_OPTIMISTIC)))
return;
saddr = &addr_buf;
}
@@ -629,8 +631,7 @@ void ndisc_send_rs(struct net_device *dev, const struct in6_addr *saddr,
int optlen = 0;
#ifdef CONFIG_IPV6_OPTIMISTIC_DAD
- /*
- * According to section 2.2 of RFC 4429, we must not
+ /* According to section 2.2 of RFC 4429, we must not
* send router solicitations with a sllao from
* optimistic addresses, but we may send the solicitation
* if we don't include the sllao. So here we check
@@ -641,9 +642,8 @@ void ndisc_send_rs(struct net_device *dev, const struct in6_addr *saddr,
struct inet6_ifaddr *ifp = ipv6_get_ifaddr(dev_net(dev), saddr,
dev, 1);
if (ifp) {
- if (ifp->flags & IFA_F_OPTIMISTIC) {
+ if (ifp->flags & IFA_F_OPTIMISTIC)
send_sllao = 0;
- }
in6_ifa_put(ifp);
} else {
send_sllao = 0;
@@ -672,11 +672,9 @@ void ndisc_send_rs(struct net_device *dev, const struct in6_addr *saddr,
ndisc_send_skb(skb, daddr, saddr);
}
-
static void ndisc_error_report(struct neighbour *neigh, struct sk_buff *skb)
{
- /*
- * "The sender MUST return an ICMP
+ /* "The sender MUST return an ICMP
* destination unreachable"
*/
dst_link_failure(skb);
@@ -695,7 +693,7 @@ static void ndisc_solicit(struct neighbour *neigh, struct sk_buff *skb)
if (skb && ipv6_chk_addr_and_flags(dev_net(dev), &ipv6_hdr(skb)->saddr,
dev, 1,
- IFA_F_TENTATIVE|IFA_F_OPTIMISTIC))
+ IFA_F_TENTATIVE | IFA_F_OPTIMISTIC))
saddr = &ipv6_hdr(skb)->saddr;
probes -= NEIGH_VAR(neigh->parms, UCAST_PROBES);
if (probes < 0) {
@@ -705,11 +703,14 @@ static void ndisc_solicit(struct neighbour *neigh, struct sk_buff *skb)
__func__, target);
}
ndisc_send_ns(dev, target, target, saddr, 0);
- } else if ((probes -= NEIGH_VAR(neigh->parms, APP_PROBES)) < 0) {
- neigh_app_ns(neigh);
} else {
- addrconf_addr_solict_mult(target, &mcaddr);
- ndisc_send_ns(dev, target, &mcaddr, saddr, 0);
+ probes -= NEIGH_VAR(neigh->parms, APP_PROBES);
+ if (probes < 0) {
+ neigh_app_ns(neigh);
+ } else {
+ addrconf_addr_solict_mult(target, &mcaddr);
+ ndisc_send_ns(dev, target, &mcaddr, saddr, 0);
+ }
}
}
@@ -765,8 +766,7 @@ static void ndisc_recv_ns(struct sk_buff *skb)
return;
}
- /*
- * RFC2461 7.1.1:
+ /* RFC2461 7.1.1:
* DAD has to be destined for solicited node multicast address.
*/
if (dad && !ipv6_addr_is_solict_mult(daddr)) {
@@ -806,7 +806,7 @@ static void ndisc_recv_ns(struct sk_buff *skb)
ifp = ipv6_get_ifaddr(dev_net(dev), &msg->target, dev, 1);
if (ifp) {
have_ifp:
- if (ifp->flags & (IFA_F_TENTATIVE|IFA_F_OPTIMISTIC)) {
+ if (ifp->flags & (IFA_F_TENTATIVE | IFA_F_OPTIMISTIC)) {
if (dad) {
if (nonce != 0 && ifp->dad_nonce == nonce) {
u8 *np = (u8 *)&nonce;
@@ -817,23 +817,21 @@ static void ndisc_recv_ns(struct sk_buff *skb)
&ifp->addr, np);
goto out;
}
- /*
- * We are colliding with another node
+ /* We are colliding with another node
* who is doing DAD
* so fail our DAD process
*/
addrconf_dad_failure(ifp);
return;
- } else {
- /*
- * This is not a dad solicitation.
- * If we are an optimistic node,
- * we should respond.
- * Otherwise, we should ignore it.
- */
- if (!(ifp->flags & IFA_F_OPTIMISTIC))
- goto out;
}
+
+ /* This is not a dad solicitation.
+ * If we are an optimistic node,
+ * we should respond.
+ * Otherwise, we should ignore it.
+ */
+ if (!(ifp->flags & IFA_F_OPTIMISTIC))
+ goto out;
}
idev = ifp->idev;
@@ -846,7 +844,8 @@ static void ndisc_recv_ns(struct sk_buff *skb)
mdev = netdev_master_upper_dev_get_rcu(dev);
if (mdev) {
- ifp = ipv6_get_ifaddr(net, &msg->target, mdev, 1);
+ ifp = ipv6_get_ifaddr(net, &msg->target,
+ mdev, 1);
if (ifp)
goto have_ifp;
}
@@ -858,28 +857,31 @@ static void ndisc_recv_ns(struct sk_buff *skb)
return;
}
+ is_router = pndisc_is_router(&msg->target, dev);
if (ipv6_chk_acast_addr(net, dev, &msg->target) ||
(idev->cnf.forwarding &&
- (net->ipv6.devconf_all->proxy_ndp || idev->cnf.proxy_ndp) &&
- (is_router = pndisc_is_router(&msg->target, dev)) >= 0)) {
+ (net->ipv6.devconf_all->proxy_ndp ||
+ idev->cnf.proxy_ndp) && is_router >= 0)) {
if (!(NEIGH_CB(skb)->flags & LOCALLY_ENQUEUED) &&
skb->pkt_type != PACKET_HOST &&
inc &&
NEIGH_VAR(idev->nd_parms, PROXY_DELAY) != 0) {
- /*
- * for anycast or proxy,
+ /* for anycast or proxy,
* sender should delay its response
* by a random time between 0 and
* MAX_ANYCAST_DELAY_TIME seconds.
* (RFC2461) -- yoshfuji
*/
struct sk_buff *n = skb_clone(skb, GFP_ATOMIC);
+
if (n)
- pneigh_enqueue(&nd_tbl, idev->nd_parms, n);
+ pneigh_enqueue(&nd_tbl,
+ idev->nd_parms, n);
goto out;
}
- } else
+ } else {
goto out;
+ }
}
if (is_router < 0)
@@ -887,7 +889,7 @@ static void ndisc_recv_ns(struct sk_buff *skb)
if (dad) {
ndisc_send_na(dev, &in6addr_linklocal_allnodes, &msg->target,
- !!is_router, false, (ifp != NULL), true);
+ !!is_router, false, ifp, true);
goto out;
}
@@ -896,20 +898,19 @@ static void ndisc_recv_ns(struct sk_buff *skb)
else
NEIGH_CACHE_STAT_INC(&nd_tbl, rcv_probes_ucast);
- /*
- * update / create cache entry
+ /* update / create cache entry
* for the source address
*/
neigh = __neigh_lookup(&nd_tbl, saddr, dev,
!inc || lladdr || !dev->addr_len);
if (neigh)
ndisc_update(dev, neigh, lladdr, NUD_STALE,
- NEIGH_UPDATE_F_WEAK_OVERRIDE|
+ NEIGH_UPDATE_F_WEAK_OVERRIDE |
NEIGH_UPDATE_F_OVERRIDE,
NDISC_NEIGHBOUR_SOLICITATION, &ndopts);
if (neigh || !dev->header_ops) {
ndisc_send_na(dev, saddr, &msg->target, !!is_router,
- true, (ifp != NULL && inc), inc);
+ true, (ifp && inc), inc);
if (neigh)
neigh_release(neigh);
}
@@ -973,19 +974,19 @@ static void ndisc_recv_na(struct sk_buff *skb)
}
ifp = ipv6_get_ifaddr(dev_net(dev), &msg->target, dev, 1);
if (ifp) {
- if (skb->pkt_type != PACKET_LOOPBACK
- && (ifp->flags & IFA_F_TENTATIVE)) {
- addrconf_dad_failure(ifp);
- return;
+ if (skb->pkt_type != PACKET_LOOPBACK &&
+ (ifp->flags & IFA_F_TENTATIVE)) {
+ addrconf_dad_failure(ifp);
+ return;
}
/* What should we make now? The advertisement
- is invalid, but ndisc specs say nothing
- about it. It could be misconfiguration, or
- an smart proxy agent tries to help us :-)
-
- We should not print the error if NA has been
- received from loopback - it is just our own
- unsolicited advertisement.
+ * is invalid, but ndisc specs say nothing
+ * about it. It could be misconfiguration, or
+ * an smart proxy agent tries to help us :-)
+ *
+ * We should not print the error if NA has been
+ * received from loopback - it is just our own
+ * unsolicited advertisement.
*/
if (skb->pkt_type != PACKET_LOOPBACK)
ND_PRINTK(1, warn,
@@ -1003,30 +1004,31 @@ static void ndisc_recv_na(struct sk_buff *skb)
if (neigh->nud_state & NUD_FAILED)
goto out;
- /*
- * Don't update the neighbor cache entry on a proxy NA from
+ /* Don't update the neighbor cache entry on a proxy NA from
* ourselves because either the proxied node is off link or it
* has already sent a NA to us.
*/
if (lladdr && !memcmp(lladdr, dev->dev_addr, dev->addr_len) &&
- net->ipv6.devconf_all->forwarding && net->ipv6.devconf_all->proxy_ndp &&
+ net->ipv6.devconf_all->forwarding &&
+ net->ipv6.devconf_all->proxy_ndp &&
pneigh_lookup(&nd_tbl, net, &msg->target, dev, 0)) {
/* XXX: idev->cnf.proxy_ndp */
goto out;
}
ndisc_update(dev, neigh, lladdr,
- msg->icmph.icmp6_solicited ? NUD_REACHABLE : NUD_STALE,
- NEIGH_UPDATE_F_WEAK_OVERRIDE|
- (msg->icmph.icmp6_override ? NEIGH_UPDATE_F_OVERRIDE : 0)|
- NEIGH_UPDATE_F_OVERRIDE_ISROUTER|
- (msg->icmph.icmp6_router ? NEIGH_UPDATE_F_ISROUTER : 0),
+ msg->icmph.icmp6_solicited ?
+ NUD_REACHABLE : NUD_STALE,
+ NEIGH_UPDATE_F_WEAK_OVERRIDE |
+ (msg->icmph.icmp6_override ?
+ NEIGH_UPDATE_F_OVERRIDE : 0) |
+ NEIGH_UPDATE_F_OVERRIDE_ISROUTER |
+ (msg->icmph.icmp6_router ?
+ NEIGH_UPDATE_F_ISROUTER : 0),
NDISC_NEIGHBOUR_ADVERTISEMENT, &ndopts);
if ((old_flags & ~neigh->flags) & NTF_ROUTER) {
- /*
- * Change: router to host
- */
+ /* Change: router to host */
rt6_clean_tohost(dev_net(dev), saddr);
}
@@ -1058,8 +1060,7 @@ static void ndisc_recv_rs(struct sk_buff *skb)
if (!idev->cnf.forwarding)
goto out;
- /*
- * Don't update NCE if src = ::;
+ /* Don't update NCE if src = ::;
* this implies that the source node has no ip address assigned yet.
*/
if (ipv6_addr_any(saddr))
@@ -1081,8 +1082,8 @@ static void ndisc_recv_rs(struct sk_buff *skb)
neigh = __neigh_lookup(&nd_tbl, saddr, skb->dev, 1);
if (neigh) {
ndisc_update(skb->dev, neigh, lladdr, NUD_STALE,
- NEIGH_UPDATE_F_WEAK_OVERRIDE|
- NEIGH_UPDATE_F_OVERRIDE|
+ NEIGH_UPDATE_F_WEAK_OVERRIDE |
+ NEIGH_UPDATE_F_OVERRIDE |
NEIGH_UPDATE_F_OVERRIDE_ISROUTER,
NDISC_ROUTER_SOLICITATION, &ndopts);
neigh_release(neigh);
@@ -1110,9 +1111,8 @@ static void ndisc_ra_useropt(struct sk_buff *ra, struct nd_opt_hdr *opt)
}
nlh = nlmsg_put(skb, 0, 0, RTM_NEWNDUSEROPT, base_size, 0);
- if (!nlh) {
+ if (!nlh)
goto nla_put_failure;
- }
ndmsg = nlmsg_data(nlh);
ndmsg->nduseropt_family = AF_INET6;
@@ -1174,9 +1174,7 @@ static void ndisc_router_discovery(struct sk_buff *skb)
}
#endif
- /*
- * set the RA_RECV flag in the interface
- */
+ /* set the RA_RECV flag in the interface */
in6_dev = __in6_dev_get(skb->dev);
if (!in6_dev) {
@@ -1208,15 +1206,13 @@ static void ndisc_router_discovery(struct sk_buff *skb)
#endif
if (in6_dev->if_flags & IF_RS_SENT) {
- /*
- * flag that an RA was received after an RS was sent
+ /* flag that an RA was received after an RS was sent
* out on this interface.
*/
in6_dev->if_flags |= IF_RA_RCVD;
}
- /*
- * Remember the managed/otherconf flags from most recently
+ /* Remember the managed/otherconf flags from most recently
* received RA message (RFC 2462) -- yoshfuji
*/
old_if_flags = in6_dev->if_flags;
@@ -1299,77 +1295,79 @@ static void ndisc_router_discovery(struct sk_buff *skb)
}
neigh->flags |= NTF_ROUTER;
} else if (rt) {
- rt->rt6i_flags = (rt->rt6i_flags & ~RTF_PREF_MASK) | RTF_PREF(pref);
+ rt->rt6i_flags =
+ (rt->rt6i_flags & ~RTF_PREF_MASK) | RTF_PREF(pref);
}
if (rt)
rt6_set_expires(rt, jiffies + (HZ * lifetime));
if (in6_dev->cnf.accept_ra_min_hop_limit < 256 &&
ra_msg->icmph.icmp6_hop_limit) {
- if (in6_dev->cnf.accept_ra_min_hop_limit <= ra_msg->icmph.icmp6_hop_limit) {
+ if (in6_dev->cnf.accept_ra_min_hop_limit <=
+ ra_msg->icmph.icmp6_hop_limit) {
in6_dev->cnf.hop_limit = ra_msg->icmph.icmp6_hop_limit;
if (rt)
dst_metric_set(&rt->dst, RTAX_HOPLIMIT,
ra_msg->icmph.icmp6_hop_limit);
} else {
- ND_PRINTK(2, warn, "RA: Got route advertisement with lower hop_limit than minimum\n");
+ ND_PRINTK(2, warn,
+ "RA: Got route advertisement with lower hop_limit than minimum\n");
}
}
skip_defrtr:
- /*
- * Update Reachable Time and Retrans Timer
- */
+ /* Update Reachable Time and Retrans Timer */
if (in6_dev->nd_parms) {
unsigned long rtime = ntohl(ra_msg->retrans_timer);
- if (rtime && rtime/1000 < MAX_SCHEDULE_TIMEOUT/HZ) {
- rtime = (rtime*HZ)/1000;
- if (rtime < HZ/10)
- rtime = HZ/10;
+ if (rtime && rtime / 1000 < MAX_SCHEDULE_TIMEOUT / HZ) {
+ rtime = (rtime * HZ) / 1000;
+ if (rtime < HZ / 10)
+ rtime = HZ / 10;
NEIGH_VAR_SET(in6_dev->nd_parms, RETRANS_TIME, rtime);
in6_dev->tstamp = jiffies;
send_ifinfo_notify = true;
}
rtime = ntohl(ra_msg->reachable_time);
- if (rtime && rtime/1000 < MAX_SCHEDULE_TIMEOUT/(3*HZ)) {
- rtime = (rtime*HZ)/1000;
+ if (rtime && rtime / 1000 < MAX_SCHEDULE_TIMEOUT / (3 * HZ)) {
+ rtime = (rtime * HZ) / 1000;
- if (rtime < HZ/10)
- rtime = HZ/10;
+ if (rtime < HZ / 10)
+ rtime = HZ / 10;
- if (rtime != NEIGH_VAR(in6_dev->nd_parms, BASE_REACHABLE_TIME)) {
+ if (rtime != NEIGH_VAR(in6_dev->nd_parms,
+ BASE_REACHABLE_TIME)) {
NEIGH_VAR_SET(in6_dev->nd_parms,
BASE_REACHABLE_TIME, rtime);
NEIGH_VAR_SET(in6_dev->nd_parms,
GC_STALETIME, 3 * rtime);
- in6_dev->nd_parms->reachable_time = neigh_rand_reach_time(rtime);
+ in6_dev->nd_parms->reachable_time =
+ neigh_rand_reach_time(rtime);
in6_dev->tstamp = jiffies;
send_ifinfo_notify = true;
}
}
}
- /*
- * Send a notify if RA changed managed/otherconf flags or timer settings
+ /* Send a notify if RA changed managed/otherconf flags
+ * or timer settings
*/
if (send_ifinfo_notify)
inet6_ifinfo_notify(RTM_NEWLINK, in6_dev);
skip_linkparms:
- /*
- * Process options.
- */
+ /* Process options */
if (!neigh)
neigh = __neigh_lookup(&nd_tbl, &ipv6_hdr(skb)->saddr,
skb->dev, 1);
if (neigh) {
u8 *lladdr = NULL;
+
if (ndopts.nd_opts_src_lladdr) {
lladdr = ndisc_opt_addr_data(ndopts.nd_opts_src_lladdr,
skb->dev);
@@ -1380,9 +1378,9 @@ static void ndisc_router_discovery(struct sk_buff *skb)
}
}
ndisc_update(skb->dev, neigh, lladdr, NUD_STALE,
- NEIGH_UPDATE_F_WEAK_OVERRIDE|
- NEIGH_UPDATE_F_OVERRIDE|
- NEIGH_UPDATE_F_OVERRIDE_ISROUTER|
+ NEIGH_UPDATE_F_WEAK_OVERRIDE |
+ NEIGH_UPDATE_F_OVERRIDE |
+ NEIGH_UPDATE_F_OVERRIDE_ISROUTER |
NEIGH_UPDATE_F_ISROUTER,
NDISC_ROUTER_ADVERTISEMENT, &ndopts);
}
@@ -1406,6 +1404,7 @@ static void ndisc_router_discovery(struct sk_buff *skb)
if (in6_dev->cnf.accept_ra_rtr_pref && ndopts.nd_opts_ri) {
struct nd_opt_hdr *p;
+
for (p = ndopts.nd_opts_ri;
p;
p = ndisc_next_option(p, ndopts.nd_opts_ri_end)) {
@@ -1418,9 +1417,11 @@ static void ndisc_router_discovery(struct sk_buff *skb)
if (ri->prefix_len == 0 &&
!in6_dev->cnf.accept_ra_defrtr)
continue;
- if (ri->prefix_len < in6_dev->cnf.accept_ra_rt_info_min_plen)
+ if (ri->prefix_len <
+ in6_dev->cnf.accept_ra_rt_info_min_plen)
continue;
- if (ri->prefix_len > in6_dev->cnf.accept_ra_rt_info_max_plen)
+ if (ri->prefix_len >
+ in6_dev->cnf.accept_ra_rt_info_max_plen)
continue;
rt6_route_rcv(skb->dev, (u8 *)p, (p->nd_opt_len) << 3,
&ipv6_hdr(skb)->saddr);
@@ -1442,12 +1443,13 @@ static void ndisc_router_discovery(struct sk_buff *skb)
if (in6_dev->cnf.accept_ra_pinfo && ndopts.nd_opts_pi) {
struct nd_opt_hdr *p;
+
for (p = ndopts.nd_opts_pi;
p;
p = ndisc_next_option(p, ndopts.nd_opts_pi_end)) {
addrconf_prefix_rcv(skb->dev, (u8 *)p,
(p->nd_opt_len) << 3,
- ndopts.nd_opts_src_lladdr != NULL);
+ ndopts.nd_opts_src_lladdr);
}
}
@@ -1455,7 +1457,7 @@ static void ndisc_router_discovery(struct sk_buff *skb)
__be32 n;
u32 mtu;
- memcpy(&n, ((u8 *)(ndopts.nd_opts_mtu+1))+2, sizeof(mtu));
+ memcpy(&n, ((u8 *)(ndopts.nd_opts_mtu + 1)) + 2, sizeof(mtu));
mtu = ntohl(n);
if (mtu < IPV6_MIN_MTU || mtu > skb->dev->mtu) {
@@ -1472,6 +1474,7 @@ static void ndisc_router_discovery(struct sk_buff *skb)
if (ndopts.nd_useropts) {
struct nd_opt_hdr *p;
+
for (p = ndopts.nd_useropts;
p;
p = ndisc_next_useropt(skb->dev, p,
@@ -1480,9 +1483,9 @@ static void ndisc_router_discovery(struct sk_buff *skb)
}
}
- if (ndopts.nd_opts_tgt_lladdr || ndopts.nd_opts_rh) {
+ if (ndopts.nd_opts_tgt_lladdr || ndopts.nd_opts_rh)
ND_PRINTK(2, warn, "RA: invalid RA options\n");
- }
+
out:
ip6_rt_put(rt);
if (neigh)
@@ -1518,7 +1521,7 @@ static void ndisc_redirect_rcv(struct sk_buff *skb)
if (!ndopts.nd_opts_rh) {
ip6_redirect_no_header(skb, dev_net(skb->dev),
- skb->dev->ifindex, 0);
+ skb->dev->ifindex, 0);
return;
}
@@ -1569,7 +1572,8 @@ void ndisc_send_redirect(struct sk_buff *skb, const struct in6_addr *target)
}
if (!ipv6_addr_equal(&ipv6_hdr(skb)->daddr, target) &&
- ipv6_addr_type(target) != (IPV6_ADDR_UNICAST|IPV6_ADDR_LINKLOCAL)) {
+ ipv6_addr_type(target) !=
+ (IPV6_ADDR_UNICAST | IPV6_ADDR_LINKLOCAL)) {
ND_PRINTK(2, warn,
"Redirect: target address is not link-local unicast\n");
return;
@@ -1587,7 +1591,7 @@ void ndisc_send_redirect(struct sk_buff *skb, const struct in6_addr *target)
if (IS_ERR(dst))
return;
- rt = (struct rt6_info *) dst;
+ rt = (struct rt6_info *)dst;
if (rt->rt6i_flags & RTF_GATEWAY) {
ND_PRINTK(2, warn,
@@ -1595,14 +1599,16 @@ void ndisc_send_redirect(struct sk_buff *skb, const struct in6_addr *target)
goto release;
}
peer = inet_getpeer_v6(net->ipv6.peers, &ipv6_hdr(skb)->saddr, 1);
- ret = inet_peer_xrlim_allow(peer, 1*HZ);
+ ret = inet_peer_xrlim_allow(peer, 1 * HZ);
if (peer)
inet_putpeer(peer);
if (!ret)
goto release;
if (dev->addr_len) {
- struct neighbour *neigh = dst_neigh_lookup(skb_dst(skb), target);
+ struct neighbour *neigh =
+ dst_neigh_lookup(skb_dst(skb), target);
+
if (!neigh) {
ND_PRINTK(2, warn,
"Redirect: no neigh for target address\n");
@@ -1617,14 +1623,16 @@ void ndisc_send_redirect(struct sk_buff *skb, const struct in6_addr *target)
optlen += ndisc_redirect_opt_addr_space(dev, neigh,
ops_data_buf,
&ops_data);
- } else
+ } else {
read_unlock_bh(&neigh->lock);
+ }
neigh_release(neigh);
}
rd_len = min_t(unsigned int,
- IPV6_MIN_MTU - sizeof(struct ipv6hdr) - sizeof(*msg) - optlen,
+ IPV6_MIN_MTU - sizeof(struct ipv6hdr) -
+ sizeof(*msg) - optlen,
skb->len + 8);
rd_len &= ~0x7;
optlen += rd_len;
@@ -1642,16 +1650,12 @@ void ndisc_send_redirect(struct sk_buff *skb, const struct in6_addr *target)
.dest = ipv6_hdr(skb)->daddr,
};
- /*
- * include target_address option
- */
+ /* include target_address option */
if (ha)
ndisc_fill_redirect_addr_option(buff, ha, ops_data);
- /*
- * build redirect option and copy skb over to the new packet.
- */
+ /* build redirect option and copy skb over to the new packet */
if (rd_len)
ndisc_fill_redirect_hdr_option(buff, skb, rd_len);
@@ -1737,7 +1741,8 @@ int ndisc_rcv(struct sk_buff *skb)
return 0;
}
-static int ndisc_netdev_event(struct notifier_block *this, unsigned long event, void *ptr)
+static int ndisc_netdev_event(struct notifier_block *this,
+ unsigned long event, void *ptr)
{
struct net_device *dev = netdev_notifier_info_to_dev(ptr);
struct netdev_notifier_change_info *change_info;
@@ -1787,6 +1792,7 @@ static void ndisc_warn_deprecated_sysctl(struct ctl_table *ctl,
{
static char warncomm[TASK_COMM_LEN];
static int warned;
+
if (strcmp(warncomm, current->comm) && warned < 5) {
strcpy(warncomm, current->comm);
pr_warn("process `%s' is using deprecated sysctl (%s) net.ipv6.neigh.%s.%s - use net.ipv6.neigh.%s.%s_ms instead\n",
@@ -1797,7 +1803,8 @@ static void ndisc_warn_deprecated_sysctl(struct ctl_table *ctl,
}
}
-int ndisc_ifinfo_sysctl_change(struct ctl_table *ctl, int write, void __user *buffer, size_t *lenp, loff_t *ppos)
+int ndisc_ifinfo_sysctl_change(struct ctl_table *ctl, int write,
+ void __user *buffer, size_t *lenp, loff_t *ppos)
{
struct net_device *dev = ctl->extra1;
struct inet6_dev *idev;
@@ -1805,7 +1812,8 @@ int ndisc_ifinfo_sysctl_change(struct ctl_table *ctl, int write, void __user *bu
if ((strcmp(ctl->procname, "retrans_time") == 0) ||
(strcmp(ctl->procname, "base_reachable_time") == 0))
- ndisc_warn_deprecated_sysctl(ctl, "syscall", dev ? dev->name : "default");
+ ndisc_warn_deprecated_sysctl(ctl, "syscall",
+ dev ? dev->name : "default");
if (strcmp(ctl->procname, "retrans_time") == 0)
ret = neigh_proc_dointvec(ctl, write, buffer, lenp, ppos);
@@ -1821,18 +1829,22 @@ int ndisc_ifinfo_sysctl_change(struct ctl_table *ctl, int write, void __user *bu
else
ret = -1;
- if (write && ret == 0 && dev && (idev = in6_dev_get(dev)) != NULL) {
- if (ctl->data == &NEIGH_VAR(idev->nd_parms, BASE_REACHABLE_TIME))
+ if (!write || ret || !dev)
+ return ret;
+
+ idev = in6_dev_get(dev);
+ if (idev) {
+ if (ctl->data ==
+ &NEIGH_VAR(idev->nd_parms, BASE_REACHABLE_TIME))
idev->nd_parms->reachable_time =
- neigh_rand_reach_time(NEIGH_VAR(idev->nd_parms, BASE_REACHABLE_TIME));
+ neigh_rand_reach_time(
+ NEIGH_VAR(idev->nd_parms, BASE_REACHABLE_TIME));
idev->tstamp = jiffies;
inet6_ifinfo_notify(RTM_NEWLINK, idev);
in6_dev_put(idev);
}
return ret;
}
-
-
#endif
static int __net_init ndisc_net_init(struct net *net)
@@ -1877,9 +1889,7 @@ int __init ndisc_init(void)
err = register_pernet_subsys(&ndisc_net_ops);
if (err)
return err;
- /*
- * Initialize the neighbour table
- */
+ /* Initialize the neighbour table */
neigh_table_init(NEIGH_ND_TABLE, &nd_tbl);
#ifdef CONFIG_SYSCTL
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net-next v2] bridge: fix hello and hold timers starting/stopping
From: Hangbin Liu @ 2017-05-20 5:57 UTC (permalink / raw)
To: Ivan Vecera; +Cc: netdev, davem, sashok, stephen, bridge, lucien.xin, nikolay
In-Reply-To: <20170519173043.10201-1-cera@cera.cz>
On Fri, May 19, 2017 at 07:30:43PM +0200, Ivan Vecera wrote:
> Current bridge code incorrectly handles starting/stopping of hello and
> hold timers during STP enable/disable.
>
> 1. Timers are stopped in br_stp_start() during NO_STP->USER_STP
> transition. The timers are already stopped in NO_STP state so
> this is confusing no-op.
Hi Ivan,
Shouldn't we start hello timer in br_stp_start when NO_STP -> BR_KERNEL_STP ?
>
> 2. During USER_STP->NO_STP transition the timers are started. This
> does not make sense and is confusion because the timer should not be
> active in NO_STP state.
Yes, but what about BR_KERNEL_STP -> NO_STP in function br_stp_stop() ?
>
> Cc: davem@davemloft.net
> Cc: sashok@cumulusnetworks.com
> Cc: stephen@networkplumber.org
> Cc: bridge@lists.linux-foundation.org
> Cc: lucien.xin@gmail.com
> Cc: nikolay@cumulusnetworks.com
> Signed-off-by: Ivan Vecera <cera@cera.cz>
> ---
> net/bridge/br_stp_if.c | 11 -----------
> 1 file changed, 11 deletions(-)
>
> diff --git a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c
> index 08341d2aa9c9..a05027027513 100644
> --- a/net/bridge/br_stp_if.c
> +++ b/net/bridge/br_stp_if.c
> @@ -150,7 +150,6 @@ static int br_stp_call_user(struct net_bridge *br, char *arg)
>
> static void br_stp_start(struct net_bridge *br)
> {
> - struct net_bridge_port *p;
> int err = -ENOENT;
>
> if (net_eq(dev_net(br->dev), &init_net))
> @@ -169,11 +168,6 @@ static void br_stp_start(struct net_bridge *br)
> if (!err) {
> br->stp_enabled = BR_USER_STP;
> br_debug(br, "userspace STP started\n");
> -
> - /* Stop hello and hold timers */
> - del_timer(&br->hello_timer);
> - list_for_each_entry(p, &br->port_list, list)
> - del_timer(&p->hold_timer);
I'm not sure if user space daemon will send bpdu or not? In comment
76b91c32dd86 ("bridge: stp: when using userspace stp stop kernel hello and
hold timers"). Nikolay said we should not handle it with BR_USER_STP.
> } else {
> br->stp_enabled = BR_KERNEL_STP;
> br_debug(br, "using kernel STP\n");
> @@ -187,7 +181,6 @@ static void br_stp_start(struct net_bridge *br)
>
> static void br_stp_stop(struct net_bridge *br)
> {
> - struct net_bridge_port *p;
> int err;
>
> if (br->stp_enabled == BR_USER_STP) {
> @@ -196,10 +189,6 @@ static void br_stp_stop(struct net_bridge *br)
> br_err(br, "failed to stop userspace STP (%d)\n", err);
>
> /* To start timers on any ports left in blocking */
> - mod_timer(&br->hello_timer, jiffies + br->hello_time);
> - list_for_each_entry(p, &br->port_list, list)
> - mod_timer(&p->hold_timer,
> - round_jiffies(jiffies + BR_HOLD_TIME));
If we do not del hello_timer. after it expired in br_hello_timer_expired(),
Our state is br->dev->flags & IFF_UP and br->stp_enabled == NO_STP, it will
call mod_timer(&br->hello_timer, round_jiffies(jiffies + br->hello_time))
and we will keep sending bpdu message even after stp stoped.
> spin_lock_bh(&br->lock);
> br_port_state_selection(br);
> spin_unlock_bh(&br->lock);
> --
So how about just like
diff --git a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c
index d8ad73b..0198f62 100644
--- a/net/bridge/br_stp_if.c
+++ b/net/bridge/br_stp_if.c
@@ -183,6 +183,7 @@ static void br_stp_start(struct net_bridge *br)
} else {
br->stp_enabled = BR_KERNEL_STP;
br_debug(br, "using kernel STP\n");
+ mod_timer(&br->hello_timer, jiffies + br->hello_time);
/* To start timers on any ports left in blocking */
br_port_state_selection(br);
@@ -202,7 +203,6 @@ static void br_stp_stop(struct net_bridge *br)
br_err(br, "failed to stop userspace STP (%d)\n", err);
/* To start timers on any ports left in blocking */
- mod_timer(&br->hello_timer, jiffies + br->hello_time);
list_for_each_entry(p, &br->port_list, list)
mod_timer(&p->hold_timer,
round_jiffies(jiffies + BR_HOLD_TIME));
@@ -211,6 +211,7 @@ static void br_stp_stop(struct net_bridge *br)
spin_unlock_bh(&br->lock);
}
+ del_timer_sync(&br->hello_timer);
br->stp_enabled = BR_NO_STP;
}
Thanks
Hangbin
^ permalink raw reply related
* Re: [PATCH net-next 6/9] xfrm: make xfrm_dev_register static
From: Steffen Klassert @ 2017-05-20 6:48 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: davem, netdev, Stephen Hemminger
In-Reply-To: <20170519165556.483-7-sthemmin@microsoft.com>
On Fri, May 19, 2017 at 09:55:53AM -0700, Stephen Hemminger wrote:
> This function is only used in this file and should not be global.
>
> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
> ---
> net/xfrm/xfrm_device.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/xfrm/xfrm_device.c b/net/xfrm/xfrm_device.c
> index 8ec8a3fcf8d4..50ec73399b48 100644
> --- a/net/xfrm/xfrm_device.c
> +++ b/net/xfrm/xfrm_device.c
> @@ -138,7 +138,7 @@ bool xfrm_dev_offload_ok(struct sk_buff *skb, struct xfrm_state *x)
> }
> EXPORT_SYMBOL_GPL(xfrm_dev_offload_ok);
>
> -int xfrm_dev_register(struct net_device *dev)
> +static int xfrm_dev_register(struct net_device *dev)
I've applied a patch with this exact fix already to the
ipsec-next tree yesterday.
^ permalink raw reply
* Re: [PATCH net-next v2] bridge: fix hello and hold timers starting/stopping
From: Nikolay Aleksandrov @ 2017-05-20 6:55 UTC (permalink / raw)
To: Hangbin Liu, Ivan Vecera; +Cc: lucien.xin, netdev, bridge, davem
In-Reply-To: <20170520055720.GA12974@leo.usersys.redhat.com>
On 5/20/17 8:57 AM, Hangbin Liu wrote:
> On Fri, May 19, 2017 at 07:30:43PM +0200, Ivan Vecera wrote:
>> Current bridge code incorrectly handles starting/stopping of hello and
>> hold timers during STP enable/disable.
>>
>> 1. Timers are stopped in br_stp_start() during NO_STP->USER_STP
>> transition. The timers are already stopped in NO_STP state so
>> this is confusing no-op.
>
> Hi Ivan,
>
> Shouldn't we start hello timer in br_stp_start when NO_STP -> BR_KERNEL_STP ?
Please see Xin Long's recent -net patch that fixes exactly this issue.
It will answer your questions below, too.
https://patchwork.ozlabs.org/patch/764685/
>>
>> 2. During USER_STP->NO_STP transition the timers are started. This
>> does not make sense and is confusion because the timer should not be
>> active in NO_STP state.
>
> Yes, but what about BR_KERNEL_STP -> NO_STP in function br_stp_stop() ?
>>
>> Cc: davem@davemloft.net
>> Cc: sashok@cumulusnetworks.com
>> Cc: stephen@networkplumber.org
>> Cc: bridge@lists.linux-foundation.org
>> Cc: lucien.xin@gmail.com
>> Cc: nikolay@cumulusnetworks.com
>> Signed-off-by: Ivan Vecera <cera@cera.cz>
>> ---
>> net/bridge/br_stp_if.c | 11 -----------
>> 1 file changed, 11 deletions(-)
>>
>> diff --git a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c
>> index 08341d2aa9c9..a05027027513 100644
>> --- a/net/bridge/br_stp_if.c
>> +++ b/net/bridge/br_stp_if.c
>> @@ -150,7 +150,6 @@ static int br_stp_call_user(struct net_bridge *br, char *arg)
>>
>> static void br_stp_start(struct net_bridge *br)
>> {
>> - struct net_bridge_port *p;
>> int err = -ENOENT;
>>
>> if (net_eq(dev_net(br->dev), &init_net))
>> @@ -169,11 +168,6 @@ static void br_stp_start(struct net_bridge *br)
>> if (!err) {
>> br->stp_enabled = BR_USER_STP;
>> br_debug(br, "userspace STP started\n");
>> -
>> - /* Stop hello and hold timers */
>> - del_timer(&br->hello_timer);
>> - list_for_each_entry(p, &br->port_list, list)
>> - del_timer(&p->hold_timer);
>
> I'm not sure if user space daemon will send bpdu or not? In comment
> 76b91c32dd86 ("bridge: stp: when using userspace stp stop kernel hello and
> hold timers"). Nikolay said we should not handle it with BR_USER_STP >
>> } else {
>> br->stp_enabled = BR_KERNEL_STP;
>> br_debug(br, "using kernel STP\n");
>> @@ -187,7 +181,6 @@ static void br_stp_start(struct net_bridge *br)
>>
>> static void br_stp_stop(struct net_bridge *br)
>> {
>> - struct net_bridge_port *p;
>> int err;
>>
>> if (br->stp_enabled == BR_USER_STP) {
>> @@ -196,10 +189,6 @@ static void br_stp_stop(struct net_bridge *br)
>> br_err(br, "failed to stop userspace STP (%d)\n", err);
>>
>> /* To start timers on any ports left in blocking */
>> - mod_timer(&br->hello_timer, jiffies + br->hello_time);
>> - list_for_each_entry(p, &br->port_list, list)
>> - mod_timer(&p->hold_timer,
>> - round_jiffies(jiffies + BR_HOLD_TIME));
>
> If we do not del hello_timer. after it expired in br_hello_timer_expired(),
> Our state is br->dev->flags & IFF_UP and br->stp_enabled == NO_STP, it will
> call mod_timer(&br->hello_timer, round_jiffies(jiffies + br->hello_time))
> and we will keep sending bpdu message even after stp stoped.
Again see Xin Long's recent -net patch.
>
>> spin_lock_bh(&br->lock);
>> br_port_state_selection(br);
>> spin_unlock_bh(&br->lock);
>> --
>
> So how about just like
>
> diff --git a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c
> index d8ad73b..0198f62 100644
> --- a/net/bridge/br_stp_if.c
> +++ b/net/bridge/br_stp_if.c
> @@ -183,6 +183,7 @@ static void br_stp_start(struct net_bridge *br)
> } else {
> br->stp_enabled = BR_KERNEL_STP;
> br_debug(br, "using kernel STP\n");
> + mod_timer(&br->hello_timer, jiffies + br->hello_time);
>
> /* To start timers on any ports left in blocking */
> br_port_state_selection(br);
> @@ -202,7 +203,6 @@ static void br_stp_stop(struct net_bridge *br)
> br_err(br, "failed to stop userspace STP (%d)\n", err);
>
> /* To start timers on any ports left in blocking */
> - mod_timer(&br->hello_timer, jiffies + br->hello_time);
> list_for_each_entry(p, &br->port_list, list)
> mod_timer(&p->hold_timer,
> round_jiffies(jiffies + BR_HOLD_TIME));
> @@ -211,6 +211,7 @@ static void br_stp_stop(struct net_bridge *br)
> spin_unlock_bh(&br->lock);
> }
>
> + del_timer_sync(&br->hello_timer);
> br->stp_enabled = BR_NO_STP;
> }
>
> Thanks
> Hangbin
>
^ permalink raw reply
* Re: [PATCH net-next v2] bridge: fix hello and hold timers starting/stopping
From: Ivan Vecera @ 2017-05-20 7:06 UTC (permalink / raw)
To: Hangbin Liu
Cc: Xin Long, Nikolay Aleksandrov, network dev, bridge, David Miller
In-Reply-To: <20170520055720.GA12974@leo.usersys.redhat.com>
2017-05-20 7:57 GMT+02:00 Hangbin Liu <liuhangbin@gmail.com>:
> On Fri, May 19, 2017 at 07:30:43PM +0200, Ivan Vecera wrote:
>> Current bridge code incorrectly handles starting/stopping of hello and
>> hold timers during STP enable/disable.
>>
>> 1. Timers are stopped in br_stp_start() during NO_STP->USER_STP
>> transition. The timers are already stopped in NO_STP state so
>> this is confusing no-op.
>
> Hi Ivan,
>
> Shouldn't we start hello timer in br_stp_start when NO_STP -> BR_KERNEL_STP ?
As Nikolay mentioned, this is fixed by
https://patchwork.ozlabs.org/patch/764685/
>>
>> 2. During USER_STP->NO_STP transition the timers are started. This
>> does not make sense and is confusion because the timer should not be
>> active in NO_STP state.
>
> Yes, but what about BR_KERNEL_STP -> NO_STP in function br_stp_stop() ?
The timer is lazily stopped by itself in its handler... or not rearmed
respectively.
>> Cc: davem@davemloft.net
>> Cc: sashok@cumulusnetworks.com
>> Cc: stephen@networkplumber.org
>> Cc: bridge@lists.linux-foundation.org
>> Cc: lucien.xin@gmail.com
>> Cc: nikolay@cumulusnetworks.com
>> Signed-off-by: Ivan Vecera <cera@cera.cz>
>> ---
>> net/bridge/br_stp_if.c | 11 -----------
>> 1 file changed, 11 deletions(-)
>>
>> diff --git a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c
>> index 08341d2aa9c9..a05027027513 100644
>> --- a/net/bridge/br_stp_if.c
>> +++ b/net/bridge/br_stp_if.c
>> @@ -150,7 +150,6 @@ static int br_stp_call_user(struct net_bridge *br, char *arg)
>>
>> static void br_stp_start(struct net_bridge *br)
>> {
>> - struct net_bridge_port *p;
>> int err = -ENOENT;
>>
>> if (net_eq(dev_net(br->dev), &init_net))
>> @@ -169,11 +168,6 @@ static void br_stp_start(struct net_bridge *br)
>> if (!err) {
>> br->stp_enabled = BR_USER_STP;
>> br_debug(br, "userspace STP started\n");
>> -
>> - /* Stop hello and hold timers */
>> - del_timer(&br->hello_timer);
>> - list_for_each_entry(p, &br->port_list, list)
>> - del_timer(&p->hold_timer);
>
> I'm not sure if user space daemon will send bpdu or not? In comment
> 76b91c32dd86 ("bridge: stp: when using userspace stp stop kernel hello and
> hold timers"). Nikolay said we should not handle it with BR_USER_STP.
>
>> } else {
>> br->stp_enabled = BR_KERNEL_STP;
>> br_debug(br, "using kernel STP\n");
>> @@ -187,7 +181,6 @@ static void br_stp_start(struct net_bridge *br)
>>
>> static void br_stp_stop(struct net_bridge *br)
>> {
>> - struct net_bridge_port *p;
>> int err;
>>
>> if (br->stp_enabled == BR_USER_STP) {
>> @@ -196,10 +189,6 @@ static void br_stp_stop(struct net_bridge *br)
>> br_err(br, "failed to stop userspace STP (%d)\n", err);
>>
>> /* To start timers on any ports left in blocking */
>> - mod_timer(&br->hello_timer, jiffies + br->hello_time);
>> - list_for_each_entry(p, &br->port_list, list)
>> - mod_timer(&p->hold_timer,
>> - round_jiffies(jiffies + BR_HOLD_TIME));
>
> If we do not del hello_timer. after it expired in br_hello_timer_expired(),
> Our state is br->dev->flags & IFF_UP and br->stp_enabled == NO_STP, it will
> call mod_timer(&br->hello_timer, round_jiffies(jiffies + br->hello_time))
> and we will keep sending bpdu message even after stp stoped.
>
>> spin_lock_bh(&br->lock);
>> br_port_state_selection(br);
>> spin_unlock_bh(&br->lock);
>> --
>
> So how about just like
>
> diff --git a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c
> index d8ad73b..0198f62 100644
> --- a/net/bridge/br_stp_if.c
> +++ b/net/bridge/br_stp_if.c
> @@ -183,6 +183,7 @@ static void br_stp_start(struct net_bridge *br)
> } else {
> br->stp_enabled = BR_KERNEL_STP;
> br_debug(br, "using kernel STP\n");
> + mod_timer(&br->hello_timer, jiffies + br->hello_time);
>
> /* To start timers on any ports left in blocking */
> br_port_state_selection(br);
> @@ -202,7 +203,6 @@ static void br_stp_stop(struct net_bridge *br)
> br_err(br, "failed to stop userspace STP (%d)\n", err);
>
> /* To start timers on any ports left in blocking */
> - mod_timer(&br->hello_timer, jiffies + br->hello_time);
> list_for_each_entry(p, &br->port_list, list)
> mod_timer(&p->hold_timer,
> round_jiffies(jiffies + BR_HOLD_TIME));
> @@ -211,6 +211,7 @@ static void br_stp_stop(struct net_bridge *br)
> spin_unlock_bh(&br->lock);
> }
>
> + del_timer_sync(&br->hello_timer);
> br->stp_enabled = BR_NO_STP;
> }
>
> Thanks
> Hangbin
^ 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