* [PATCH v2 net-next] net: ethernet: ti: cpdma: correct error handling for chan create
From: Ivan Khoronzhuk @ 2017-12-12 16:35 UTC (permalink / raw)
To: grygorii.strashko
Cc: netdev, davem, linux-omap, linux-kernel, Ivan Khoronzhuk
It's not correct to return NULL when that is actually an error and
function returns errors in any other wrong case. In the same time,
the cpsw driver and davinci emac doesn't check error case while
creating channel and it can miss actual error. Also remove WARNs
duplicated dev_err msgs.
Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
---
drivers/net/ethernet/ti/cpsw.c | 12 +++++++++---
drivers/net/ethernet/ti/davinci_cpdma.c | 2 +-
drivers/net/ethernet/ti/davinci_emac.c | 9 +++++++--
3 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index a60a378..3c85a08 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -3065,10 +3065,16 @@ static int cpsw_probe(struct platform_device *pdev)
}
cpsw->txv[0].ch = cpdma_chan_create(cpsw->dma, 0, cpsw_tx_handler, 0);
+ if (IS_ERR(cpsw->txv[0].ch)) {
+ dev_err(priv->dev, "error initializing tx dma channel\n");
+ ret = PTR_ERR(cpsw->txv[0].ch);
+ goto clean_dma_ret;
+ }
+
cpsw->rxv[0].ch = cpdma_chan_create(cpsw->dma, 0, cpsw_rx_handler, 1);
- if (WARN_ON(!cpsw->rxv[0].ch || !cpsw->txv[0].ch)) {
- dev_err(priv->dev, "error initializing dma channels\n");
- ret = -ENOMEM;
+ if (IS_ERR(cpsw->rxv[0].ch)) {
+ dev_err(priv->dev, "error initializing rx dma channel\n");
+ ret = PTR_ERR(cpsw->rxv[0].ch);
goto clean_dma_ret;
}
diff --git a/drivers/net/ethernet/ti/davinci_cpdma.c b/drivers/net/ethernet/ti/davinci_cpdma.c
index e4d6edf..6f9173f 100644
--- a/drivers/net/ethernet/ti/davinci_cpdma.c
+++ b/drivers/net/ethernet/ti/davinci_cpdma.c
@@ -893,7 +893,7 @@ struct cpdma_chan *cpdma_chan_create(struct cpdma_ctlr *ctlr, int chan_num,
chan_num = rx_type ? rx_chan_num(chan_num) : tx_chan_num(chan_num);
if (__chan_linear(chan_num) >= ctlr->num_chan)
- return NULL;
+ return ERR_PTR(-EINVAL);
chan = devm_kzalloc(ctlr->dev, sizeof(*chan), GFP_KERNEL);
if (!chan)
diff --git a/drivers/net/ethernet/ti/davinci_emac.c b/drivers/net/ethernet/ti/davinci_emac.c
index f58c0c6..3d4af64 100644
--- a/drivers/net/ethernet/ti/davinci_emac.c
+++ b/drivers/net/ethernet/ti/davinci_emac.c
@@ -1870,10 +1870,15 @@ static int davinci_emac_probe(struct platform_device *pdev)
priv->txchan = cpdma_chan_create(priv->dma, EMAC_DEF_TX_CH,
emac_tx_handler, 0);
+ if (WARN_ON(IS_ERR(priv->txchan))) {
+ rc = PTR_ERR(priv->txchan);
+ goto no_cpdma_chan;
+ }
+
priv->rxchan = cpdma_chan_create(priv->dma, EMAC_DEF_RX_CH,
emac_rx_handler, 1);
- if (WARN_ON(!priv->txchan || !priv->rxchan)) {
- rc = -ENOMEM;
+ if (WARN_ON(IS_ERR(priv->rxchan))) {
+ rc = PTR_ERR(priv->rxchan);
goto no_cpdma_chan;
}
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net] tcp md5sig: Use skb's saddr when replying to an incoming segment
From: David Miller @ 2017-12-12 16:21 UTC (permalink / raw)
To: cpaasch; +Cc: netdev
In-Reply-To: <20171211080546.89418-1-cpaasch@apple.com>
From: Christoph Paasch <cpaasch@apple.com>
Date: Mon, 11 Dec 2017 00:05:46 -0800
> The MD5-key that belongs to a connection is identified by the peer's
> IP-address. When we are in tcp_v4(6)_reqsk_send_ack(), we are replying
> to an incoming segment from tcp_check_req() that failed the seq-number
> checks.
>
> Thus, to find the correct key, we need to use the skb's saddr and not
> the daddr.
>
> This bug seems to have been there since quite a while, but probably got
> unnoticed because the consequences are not catastrophic. We will call
> tcp_v4_reqsk_send_ack only to send a challenge-ACK back to the peer,
> thus the connection doesn't really fail.
>
> Fixes: 9501f9722922 ("tcp md5sig: Let the caller pass appropriate key for tcp_v{4,6}_do_calc_md5_hash().")
> Signed-off-by: Christoph Paasch <cpaasch@apple.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* [PATCH] openvswitch: Trim off padding before L3 conntrack processing
From: Ed Swierk @ 2017-12-12 16:17 UTC (permalink / raw)
To: ovs-dev, netdev, Pravin Shelar
Cc: Lance Richardson, Ed Swierk, Benjamin Warren, Keith Holleman
A short IPv4 packet may have up to 6 bytes of padding following the IP
payload when received on an Ethernet device.
In the normal IPv4 receive path, ip_rcv() trims the packet to
ip_hdr->tot_len before invoking NF_INET_PRE_ROUTING hooks (including
conntrack). Then any subsequent L3+ processing steps, like
nf_checksum(), use skb->len as the length of the packet, rather than
referring back to ip_hdr->tot_len. In the IPv6 receive path, ip6_rcv()
does the same using ipv6_hdr->payload_len.
In the OVS conntrack receive path, this trimming does not occur, so
the checksum verification in tcp_header() fails, printing "nf_ct_tcp:
bad TCP checksum". Extra zero bytes don't affect the checksum, but the
length in the IP pseudoheader does. That length is based on skb->len,
and without trimming, it doesn't match the length the sender used when
computing the checksum.
With this change, OVS conntrack trims IPv4 and IPv6 packets prior to
L3 processing.
Signed-off-by: Ed Swierk <eswierk@skyportsystems.com>
---
net/openvswitch/conntrack.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index d558e882ca0c..3a7c9215c431 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -1105,12 +1105,29 @@ int ovs_ct_execute(struct net *net, struct sk_buff *skb,
const struct ovs_conntrack_info *info)
{
int nh_ofs;
+ unsigned int nh_len;
int err;
/* The conntrack module expects to be working at L3. */
nh_ofs = skb_network_offset(skb);
skb_pull_rcsum(skb, nh_ofs);
+ /* Trim to L3 length since nf_checksum() doesn't expect padding. */
+ switch (skb->protocol) {
+ case htons(ETH_P_IP):
+ nh_len = ntohs(ip_hdr(skb)->tot_len);
+ break;
+ case htons(ETH_P_IPV6):
+ nh_len = ntohs(ipv6_hdr(skb)->payload_len)
+ + sizeof(struct ipv6hdr);
+ break;
+ default:
+ nh_len = skb->len;
+ }
+ err = pskb_trim_rcsum(skb, nh_len);
+ if (err)
+ return err;
+
if (key->ip.frag != OVS_FRAG_TYPE_NONE) {
err = handle_fragments(net, key, info->zone.id, skb);
if (err)
--
2.11.0
^ permalink raw reply related
* [PATCH] veth: Optionally pad packets to minimum Ethernet length
From: Ed Swierk @ 2017-12-12 16:13 UTC (permalink / raw)
To: netdev; +Cc: Ed Swierk, Benjamin Warren, Keith Holleman
Most physical Ethernet devices pad short packets to the minimum length
of 64 bytes (including FCS) on transmit. It can be useful to simulate
this behavior when debugging a problem that results from it (such as
incorrect L4 checksum calculation).
Padding is unnecessary for most applications so leave it off by
default. Enable padding only when the otherwise unused IFF_AUTOMEDIA
flag is set (e.g. by writing 0x5003 to flags in sysfs).
Signed-off-by: Ed Swierk <eswierk@skyportsystems.com>
---
drivers/net/veth.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index f5438d0978ca..292029bf4bb2 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -111,6 +111,12 @@ static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev)
goto drop;
}
+ if (unlikely(dev->flags & IFF_AUTOMEDIA)) {
+ /* if eth_skb_pad returns an error the skb was freed */
+ if (eth_skb_pad(skb))
+ goto drop;
+ }
+
if (likely(dev_forward_skb(rcv, skb) == NET_RX_SUCCESS)) {
struct pcpu_vstats *stats = this_cpu_ptr(dev->vstats);
--
2.11.0
^ permalink raw reply related
* Re: [PATCH 2/4] sctp: Add ip option support
From: Marcelo Ricardo Leitner @ 2017-12-12 16:08 UTC (permalink / raw)
To: Richard Haines
Cc: selinux, netdev, linux-sctp, linux-security-module, paul,
vyasevich, nhorman, sds, eparis
In-Reply-To: <20171127193121.2666-1-richard_c_haines@btinternet.com>
Hi Richard,
On Mon, Nov 27, 2017 at 07:31:21PM +0000, Richard Haines wrote:
...
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -3123,8 +3123,10 @@ static int sctp_setsockopt_maxseg(struct sock *sk, char __user *optval, unsigned
>
> if (asoc) {
> if (val == 0) {
> + struct sctp_af *af = sp->pf->af;
> val = asoc->pathmtu;
> - val -= sp->pf->af->net_header_len;
> + val -= af->ip_options_len(asoc->base.sk);
> + val -= af->net_header_len;
> val -= sizeof(struct sctphdr) +
> sizeof(struct sctp_data_chunk);
> }
Right below here there is a call to sctp_frag_point(). That function
also needs this tweak.
Yes, we should simplify all these calculations. I have a patch to use
sctp_frag_point on where it is currently recalculating it on
sctp_datamsg_from_user(), but probably should include other places as
well.
Marcelo
^ permalink raw reply
* Re: [PATCH net-next v4 1/5] net: Introduce NETIF_F_GRO_HW.
From: Alexander Duyck @ 2017-12-12 15:57 UTC (permalink / raw)
To: Michael Chan
Cc: David Miller, Netdev, Andrew Gospodarek, Ariel Elior,
everest-linux-l2
In-Reply-To: <1512992470-28861-2-git-send-email-michael.chan@broadcom.com>
On Mon, Dec 11, 2017 at 3:41 AM, Michael Chan <michael.chan@broadcom.com> wrote:
> Introduce NETIF_F_GRO_HW feature flag for NICs that support hardware
> GRO. With this flag, we can now independently turn on or off hardware
> GRO when GRO is on. Previously, drivers were using NETIF_F_GRO to
> control hardware GRO and so it cannot be independently turned on or
> off without affecting GRO.
>
> Hardware GRO (just like GRO) guarantees that packets can be re-segmented
> by TSO/GSO to reconstruct the original packet stream. Logically,
> GRO_HW should depend on GRO since it a subset, but we will let
> individual drivers enforce this dependency as they see fit.
>
> Since NETIF_F_GRO is not propagated between upper and lower devices,
> NETIF_F_GRO_HW should follow suit since it is a subset of GRO. In other
> words, a lower device can independent have GRO/GRO_HW enabled or disabled
> and no feature propagation is required. This will preserve the current
> GRO behavior. This can be changed later if we decide to propagate GRO/
> GRO_HW/RXCSUM from upper to lower devices.
>
> Cc: Ariel Elior <Ariel.Elior@cavium.com>
> Cc: everest-linux-l2@cavium.com
> Signed-off-by: Michael Chan <michael.chan@broadcom.com>
> ---
> Documentation/networking/netdev-features.txt | 8 ++++++++
> include/linux/netdev_features.h | 3 +++
> net/core/dev.c | 10 ++++++++++
> net/core/ethtool.c | 1 +
> 4 files changed, 22 insertions(+)
>
> diff --git a/Documentation/networking/netdev-features.txt b/Documentation/networking/netdev-features.txt
> index 7413eb0..8f36527 100644
> --- a/Documentation/networking/netdev-features.txt
> +++ b/Documentation/networking/netdev-features.txt
> @@ -163,3 +163,11 @@ This requests that the NIC receive all possible frames, including errored
> frames (such as bad FCS, etc). This can be helpful when sniffing a link with
> bad packets on it. Some NICs may receive more packets if also put into normal
> PROMISC mode.
> +
> +* rx-gro-hw
> +
> +This requests that the NIC enables Hardware GRO (generic receive offload).
> +Hardware GRO is basically the exact reverse of TSO, and is generally
> +stricter than Hardware LRO. A packet stream merged by Hardware GRO must
> +be re-segmentable by GSO or TSO back to the exact original packet stream.
> +Hardware GRO is dependent on GRO and RXCSUM.
This last line should probably be updated since we made the change
that dropped the dependency on software GRO.
> diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
> index b1b0ca7..db84c51 100644
> --- a/include/linux/netdev_features.h
> +++ b/include/linux/netdev_features.h
> @@ -78,6 +78,8 @@ enum {
> NETIF_F_HW_ESP_TX_CSUM_BIT, /* ESP with TX checksum offload */
> NETIF_F_RX_UDP_TUNNEL_PORT_BIT, /* Offload of RX port for UDP tunnels */
>
> + NETIF_F_GRO_HW_BIT, /* Hardware Generic receive offload */
> +
> /*
> * Add your fresh new feature above and remember to update
> * netdev_features_strings[] in net/core/ethtool.c and maybe
> @@ -97,6 +99,7 @@ enum {
> #define NETIF_F_FRAGLIST __NETIF_F(FRAGLIST)
> #define NETIF_F_FSO __NETIF_F(FSO)
> #define NETIF_F_GRO __NETIF_F(GRO)
> +#define NETIF_F_GRO_HW __NETIF_F(GRO_HW)
> #define NETIF_F_GSO __NETIF_F(GSO)
> #define NETIF_F_GSO_ROBUST __NETIF_F(GSO_ROBUST)
> #define NETIF_F_HIGHDMA __NETIF_F(HIGHDMA)
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 8aa2f70..bf0149e 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -7424,6 +7424,16 @@ static netdev_features_t netdev_fix_features(struct net_device *dev,
> features &= ~dev->gso_partial_features;
> }
>
> + if (!(features & NETIF_F_RXCSUM)) {
> + /* NETIF_F_GRO_HW implies doing RXCSUM. If the user does not
> + * want to enable RXCSUM, logically, we should disable GRO_HW.
> + */
It might be in our interest here to explain why. Specifically that
"The Rx checksum can no longer be verified once a packet is modified
as a result of GRO_HW, therefore we cannot allow GRO_HW to be enabled
when RXCSUM is disabled." or something to that effect. Otherwise there
isn't a good explanation as to why GRO_HW implies this.
> + if (features & NETIF_F_GRO_HW) {
> + netdev_dbg(dev, "Dropping NETIF_F_GRO_HW since no RXCSUM feature.\n");
> + features &= ~NETIF_F_GRO_HW;
> + }
> + }
> +
> return features;
> }
>
> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
> index f8fcf45..50a7920 100644
> --- a/net/core/ethtool.c
> +++ b/net/core/ethtool.c
> @@ -73,6 +73,7 @@ int ethtool_op_get_ts_info(struct net_device *dev, struct ethtool_ts_info *info)
> [NETIF_F_LLTX_BIT] = "tx-lockless",
> [NETIF_F_NETNS_LOCAL_BIT] = "netns-local",
> [NETIF_F_GRO_BIT] = "rx-gro",
> + [NETIF_F_GRO_HW_BIT] = "rx-gro-hw",
> [NETIF_F_LRO_BIT] = "rx-lro",
>
> [NETIF_F_TSO_BIT] = "tx-tcp-segmentation",
^ permalink raw reply
* AF_VSOCK connection refused errno
From: Stefan Hajnoczi @ 2017-12-12 15:53 UTC (permalink / raw)
To: Jorgen Hansen; +Cc: Dexuan Cui, netdev
[-- Attachment #1: Type: text/plain, Size: 559 bytes --]
When connect(2) fails because the peer is not listening the virtio vsock
transport returns ECONNRESET. I believe the VMCI transport does the
same (based on code inspection).
Jorgen: Can you confirm this VMCI transport behavior?
I'd like to change to ECONNREFUSED for all transports because developers
will be surprised when they get ECONNRESET. It makes porting AF_INET
code harder.
On the other hand, it may be too late to fix this if there userspace
applications that rely on ECONNRESET? I'm not aware of any such
applications myself.
Thanks,
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply
* Re: [PATCH net-next] libbpf: add function to setup XDP
From: David Miller @ 2017-12-12 15:53 UTC (permalink / raw)
To: makita.toshiaki; +Cc: eric, netdev, linux-kernel, ast, daniel
In-Reply-To: <c1be066a-eb39-c9e0-b21b-4bb4da280b26@lab.ntt.co.jp>
From: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Date: Mon, 11 Dec 2017 11:24:12 +0900
> On 2017/12/09 23:43, Eric Leblond wrote:
>> Most of the code is taken from set_link_xdp_fd() in bpf_load.c and
>> slightly modified to be library compliant.
>>
>> Signed-off-by: Eric Leblond <eric@regit.org>
> ...
>> +int bpf_set_link_xdp_fd(int ifindex, int fd, __u32 flags)
> ...
>> + for (nh = (struct nlmsghdr *)buf; NLMSG_OK(nh, len);
>> + nh = NLMSG_NEXT(nh, len)) {
>> + if (nh->nlmsg_pid != getpid()) {
>
> Generally nlmsg_pid should not be compared with process id.
> See man netlink and
> https://github.com/iovisor/bcc/pull/1275/commits/69ce96a54c55960c8de3392061254c97b6306a6d
Right. I wish we had never named this "pid", it gets misinterpreted
way too easily.
^ permalink raw reply
* gso_type field for LRO packets
From: Gal Pressman @ 2017-12-12 15:53 UTC (permalink / raw)
To: netdev@vger.kernel.org, Eric Dumazet, David S. Miller,
Ben Hutchings, Michael S. Tsirkin
Cc: Tariq Toukan, Saeed Mahameed
Hi all,
I've been exploring hardware LRO related code in the stack and came across an inconsistency I don't quite understand.
When receiving a hardware LRO packet, most of the drivers set skb_shinfo(skb)->gso_type (For example [1]) in order to prevent macvtap from breaking.
According to include/linux/skbuff.h +/skb_warn_if_lro, LRO should set gso_size but not gso_type, which contradicts the drivers implementation and makes the check redundant.
Can anyone advise what is the correct driver implementation? should we change skb_warn_if_lro() implementation?
Regards,
Gal
[1] commit bd69ba798e21 ("qlcnic: set gso_type")
^ permalink raw reply
* Re: [PATCH net-next 0/3] tcp: better receiver autotuning
From: David Miller @ 2017-12-12 15:53 UTC (permalink / raw)
To: edumazet; +Cc: ncardwell, ycheng, soheil, weiwan, priyarjha, netdev,
eric.dumazet
In-Reply-To: <20171211015504.26551-1-edumazet@google.com>
From: Eric Dumazet <edumazet@google.com>
Date: Sun, 10 Dec 2017 17:55:01 -0800
> Now TCP senders no longer backoff when a drop is detected,
> it appears we are very often receive window limited.
>
> This series makes tcp_rcv_space_adjust() slightly more robust
> and responsive.
Series applied, thanks.
^ permalink raw reply
* [PATCH iproute2] tc: bash-completion: add missing 'classid' keyword
From: Davide Caratti @ 2017-12-12 15:45 UTC (permalink / raw)
To: Stephen Hemminger, netdev; +Cc: Yotam Gigi
users of 'matchall' filter can specify a value for the class id: update
bash-completion accordingly.
Fixes: b32c0b64fa2b ("tc: bash-completion: Add support for matchall")
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
bash-completion/tc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/bash-completion/tc b/bash-completion/tc
index 80d12972..29bca5d9 100644
--- a/bash-completion/tc
+++ b/bash-completion/tc
@@ -450,7 +450,7 @@ _tc_filter_options()
return 0
;;
matchall)
- _tc_once_attr 'action skip_sw skip_hw'
+ _tc_once_attr 'action classid skip_sw skip_hw'
return 0
;;
flower)
--
2.13.6
^ permalink raw reply related
* Re: [RFC PATCH 0/9] ethtool netlink interface (WiP)
From: Roopa Prabhu @ 2017-12-12 15:32 UTC (permalink / raw)
To: Michal Kubecek; +Cc: netdev, linux-kernel
In-Reply-To: <cover.1513000306.git.mkubecek@suse.cz>
On Mon, Dec 11, 2017 at 5:53 AM, Michal Kubecek <mkubecek@suse.cz> wrote:
> This is still work in progress and only a very small part of the ioctl
> interface is reimplemented but I would like to get some comments before
> the patchset becomes too big and changing things becomes too tedious.
>
> The interface used for communication between ethtool and kernel is based on
> ioctl() and suffers from many problems. The most pressing seems the be the
> lack of extensibility. While some of the newer commands use structures
> designed to allow future extensions (e.g. GFEATURES or TEST), most either
> allow no extension at all (GPAUSEPARAM, GCOALESCE) or only limited set of
> reserved fields (GDRVINFO, GEEE). Even most of those which support future
> extensions limit the data types that can be used.
>
> This series aims to provide an alternative interface based on netlink which
> is what other network configuration utilities use. In particular, it uses
> generic netlink (family "ethtool"). The goal is to provide an interface
> which would be extensible, flexible and practical both for ethtool and for
> other network configuration tools (e.g. wicked or systemd-networkd).
>
> The interface is documented in Documentation/networking/ethtool-netlink.txt
>
> I'll post RFC patch series for ethtool in a few days, it still needs some
> more polishing.
>
> Basic concepts:
>
> - the interface is based on generic netlink (family name "ethtool")
>
> - provide everything ioctl can do but allow easier future extensions
>
> - inextensibility of ioctl interface resulted in way too many commands,
> many of them obsoleted by newer ones; reduce the number by ignoring the
> obsolete commands and grouping some together
>
> - for "set" type commands, netlink allows providing only the attributes to
> be changed; therefore we don't need a get-modify-set cycle, userspace can
> simply say what it wants to change and how
>
> - be less dependent on ethtool and kernel being in sync; allow e.g. saying
> "ethtool -s eth0 advertise xyz off" without knowing what "xyz" means;
> it's kernel's job to know what mode "xyz" is and if it exists and is
> supported
>
> Unresolved questions/tasks:
>
> - allow dumps for "get" type requests, e.g. listing EEE settings for all
> interfaces in current netns
>
> - find reasonable format for data transfers (e.g. eeprom dump or flash)
>
> - while the netlink interface allows easy future extensions, ethtool_ops
> interface does not; some settings could be implemented using tunables and
> accessed via relevant netlink messages (as well as tunables) from
> userspace but in the long term, something better will be needed
>
> - it would be nice if driver could provide useful error/warning messages to
> be passed to userspace via extended ACK; example: while testing, I found
> a driver which only allows values 0, 1, 3 and 10000 for certain parameter
> but the only way poor user can find out is either by trying all values or
> by checking driver source
>
> Michal Kubecek (9):
> netlink: introduce nla_put_bitfield32()
> ethtool: introduce ethtool netlink interface
> ethtool: helper functions for netlink interface
> ethtool: netlink bitset handling
> ethtool: implement GET_DRVINFO message
> ethtool: implement GET_SETTINGS message
> ethtool: implement SET_SETTINGS message
> ethtool: implement GET_PARAMS message
> ethtool: implement SET_PARAMS message
>
Thanks for working on this!. Agree with most comments already
discussed on this thread.
I would prefer if we fold ethtool netlink into devlink since there is
already an overlap.
many reasons:
- have just one driver api for device global and per port config
(devlink already provides that)
- some of the devlink commands like port split/unsplit can already be
applied per netdev (and since you bring up network interface managers,
we are looking at getting these in network managers for switch ports)
- if we keep them separate, we will soon see that drivers will need
handlers for both devlink and ethtool
- and the overlap is going to be confusing for both drivers and
user-space
^ permalink raw reply
* Re: [PATCH net-next] net: phy: marvell10g: remove XGMII as an option for 88x3310
From: Andrew Lunn @ 2017-12-12 14:59 UTC (permalink / raw)
To: Russell King; +Cc: Florian Fainelli, netdev
In-Reply-To: <E1eOk3i-00010j-0V@rmk-PC.armlinux.org.uk>
On Tue, Dec 12, 2017 at 12:53:18PM +0000, Russell King wrote:
> Remove XGMII as an option for the 88x3310 PHY driver, as the PHY doesn't
> support XGMII's 32-bit data lanes. It supports USXGMII, which is not
> XGMII, but a single-lane serdes interface - see
> https://developer.cisco.com/site/usgmii-usxgmii/
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH net-next] tcp: Remove tcp_low_latency sysctl.
From: David Miller @ 2017-12-12 14:49 UTC (permalink / raw)
To: xiangxia.m.yue; +Cc: fw, netdev
In-Reply-To: <1513089788-2626-1-git-send-email-xiangxia.m.yue@gmail.com>
From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
Date: Tue, 12 Dec 2017 06:43:08 -0800
> The prequeue has been removed from the kernel. If we leave
> the tcp_low_latency in kernel, it may confuse the users.
> If users can't find the option, the doc may tell them why.
> So we remove also the sysctl about tcp_low_latency.
>
> Signed-off-by: Tonghao Zhang <zhangtonghao@didichuxing.com>
If you remove the sysctl, it will break scripts.
That's why we left it around.
^ permalink raw reply
* [PATCH net-next] tcp: Remove tcp_low_latency sysctl.
From: Tonghao Zhang @ 2017-12-12 14:43 UTC (permalink / raw)
To: fw; +Cc: netdev, Tonghao Zhang
The prequeue has been removed from the kernel. If we leave
the tcp_low_latency in kernel, it may confuse the users.
If users can't find the option, the doc may tell them why.
So we remove also the sysctl about tcp_low_latency.
Signed-off-by: Tonghao Zhang <zhangtonghao@didichuxing.com>
---
Documentation/networking/ip-sysctl.txt | 2 +-
net/ipv4/sysctl_net_ipv4.c | 10 ----------
2 files changed, 1 insertion(+), 11 deletions(-)
diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index 77f4de59dc9c..ac35f5b59739 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -356,7 +356,7 @@ tcp_l3mdev_accept - BOOLEAN
compiled with CONFIG_NET_L3_MASTER_DEV.
tcp_low_latency - BOOLEAN
- This is a legacy option, it has no effect anymore.
+ This is an obsolete option, it has been removed from kernel.
tcp_max_orphans - INTEGER
Maximal number of TCP sockets not attached to any user file handle,
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index 4602af6d5358..dfebbfcccb87 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -45,9 +45,6 @@ static int tcp_syn_retries_max = MAX_TCP_SYNCNT;
static int ip_ping_group_range_min[] = { 0, 0 };
static int ip_ping_group_range_max[] = { GID_T_MAX, GID_T_MAX };
-/* obsolete */
-static int sysctl_tcp_low_latency __read_mostly;
-
/* Update system visible IP port range */
static void set_local_port_range(struct net *net, int range[2])
{
@@ -438,13 +435,6 @@ static struct ctl_table ipv4_table[] = {
.extra1 = &one,
},
{
- .procname = "tcp_low_latency",
- .data = &sysctl_tcp_low_latency,
- .maxlen = sizeof(int),
- .mode = 0644,
- .proc_handler = proc_dointvec
- },
- {
.procname = "tcp_congestion_control",
.mode = 0644,
.maxlen = TCP_CA_NAME_MAX,
--
2.13.6
^ permalink raw reply related
* [PATCH net-next] net: sk_pacing_shift_update() helper
From: Eric Dumazet @ 2017-12-12 14:34 UTC (permalink / raw)
To: Felix Fietkau, David Miller
Cc: netdev, Johannes Berg, Toke Høiland-Jørgensen,
Kir Kolyshkin
In-Reply-To: <1511890429.16595.20.camel@gmail.com>
From: Eric Dumazet <edumazet@google.com>
In commit 3a9b76fd0db9 ("tcp: allow drivers to tweak TSQ logic")
I gave a code sample to set sk->sk_pacing_shift that was not complete.
Better add a helper that can be used by drivers without worries,
and maybe amended in the future.
A wifi driver might use it from its ndo_start_xmit()
Following call would setup TCP to allow up to ~8ms of queued data per
flow.
sk_pacing_shift_update(skb->sk, 7);
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/net/sock.h | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/include/net/sock.h b/include/net/sock.h
index 9155da42269208b358df8535b14dfd3dba509365..9a9047268d375496bccc954d03ec20baf4177fd3 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -2407,4 +2407,15 @@ static inline int sk_get_rmem0(const struct sock *sk, const struct proto *proto)
return *proto->sysctl_rmem;
}
+/* Default TCP Small queue budget is ~1 ms of data (1sec >> 10)
+ * Some wifi drivers need to tweak it to get more chunks.
+ * They can use this helper from their ndo_start_xmit()
+ */
+static inline void sk_pacing_shift_update(struct sock *sk, int val)
+{
+ if (!sk || !sk_fullsock(sk) || sk->sk_pacing_shift == val)
+ return;
+ sk->sk_pacing_shift = val;
+}
+
#endif /* _SOCK_H */
^ permalink raw reply related
* Re: [PATCH][next] net: phy: meson-gxl: make function meson_gxl_read_status static
From: Jerome Brunet @ 2017-12-12 14:20 UTC (permalink / raw)
To: Colin King, Andrew Lunn, Florian Fainelli, Carlo Caione,
Kevin Hilman, netdev, linux-arm-kernel, linux-amlogic
Cc: kernel-janitors, linux-kernel
In-Reply-To: <20171212130311.17185-1-colin.king@canonical.com>
On Tue, 2017-12-12 at 13:03 +0000, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> The function meson_gxl_read_status is local to the source and does
> not need to be in global scope, so make it static.
>
> Cleans up sparse warning:
> symbol 'meson_gxl_read_status' was not declared. Should it be static?
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Indeed, silly mistake, thx for spotting it.
Reviewed-by: Jerome Brunet <jbrunet@baylibre.com>
^ permalink raw reply
* Re: [REGRESSION][4.13.y][4.14.y][v4.15.y] net: reduce skb_warn_bad_offload() noise
From: David Miller @ 2017-12-12 14:10 UTC (permalink / raw)
To: willemdebruijn.kernel
Cc: gregkh, joseph.salisbury, edumazet, dvyukov, willemb, daniel,
jakub.kicinski, linux, john.fastabend, me, idosch, netdev,
linux-kernel, stable, 1715609
In-Reply-To: <CAF=yD-K5EmFr5z2xjmCZCqdprqQ5vmfxV3od9xBnxBQhY+_qgQ@mail.gmail.com>
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date: Mon, 11 Dec 2017 16:56:56 -0500
> On Mon, Dec 11, 2017 at 4:44 PM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
>> On Mon, Dec 11, 2017 at 04:25:26PM -0500, Willem de Bruijn wrote:
>>> Note that UFO was removed in 4.14 and that skb_warn_bad_offload
>>> can happen for various types of packets, so there may be multiple
>>> independent bug reports. I'm investigating two other non-UFO reports
>>> just now.
>>
>> Meta-comment, now that UFO is gone from mainline, I'm wondering if I
>> should just delete it from 4.4 and 4.9 as well. Any objections for
>> that? I'd like to make it easy to maintain these kernels for a while,
>> and having them diverge like this, with all of the issues around UFO,
>> seems like it will just make life harder for myself if I leave it in.
>>
>> Any opinions?
>
> Some of that removal had to be reverted with commit 0c19f846d582
> ("net: accept UFO datagrams from tuntap and packet") for VM live
> migration between kernels.
>
> Any backports probably should squash that in at the least. Just today
> another thread discussed that that patch may not address all open
> issues still, so it may be premature to backport at this point.
> http://lkml.kernel.org/r/<d71df64e-e65f-4db4-6f2e-c002c15fcbe4@01019freenet.de>
I would probably discourage backporting the UFO removal, at least for
now.
^ permalink raw reply
* [PATCH net-next] net: bridge: use rhashtable for fdbs
From: Nikolay Aleksandrov @ 2017-12-12 14:02 UTC (permalink / raw)
To: netdev; +Cc: bridge, roopa, stephen, srn, davem, Nikolay Aleksandrov
Before this patch the bridge used a fixed 256 element hash table which
was fine for small use cases (in my tests it starts to degrade
above 1000 entries), but it wasn't enough for medium or large
scale deployments. Modern setups have thousands of participants in a
single bridge, even only enabling vlans and adding a few thousand vlan
entries will cause a few thousand fdbs to be automatically inserted per
participating port. So we need to scale the fdb table considerably to
cope with modern workloads, and this patch converts it to use a
rhashtable for its operations thus improving the bridge scalability.
Tests show the following results (10 runs each), at up to 1000 entries
rhashtable is ~3% slower, at 2000 rhashtable is 30% faster, at 3000 it
is 2 times faster and at 30000 it is 50 times faster.
Obviously this happens because of the properties of the two constructs
and is expected, rhashtable keeps pretty much a constant time even with
10000000 entries (tested), while the fixed hash table struggles
considerably even above 10000.
As a side effect this also reduces the net_bridge struct size from 3248
bytes to 1344 bytes. Also note that the key struct is 8 bytes.
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
After this I'll post patches for the per-port fdb limit option. Later we
can get rid of hash_lock altogether though that requires much more
careful changes.
include/trace/events/bridge.h | 4 +-
net/bridge/br_device.c | 10 ++
net/bridge/br_fdb.c | 392 ++++++++++++++++++++----------------------
net/bridge/br_private.h | 16 +-
net/bridge/br_switchdev.c | 8 +-
5 files changed, 211 insertions(+), 219 deletions(-)
diff --git a/include/trace/events/bridge.h b/include/trace/events/bridge.h
index 1bee3e7fdf32..8ea966448b58 100644
--- a/include/trace/events/bridge.h
+++ b/include/trace/events/bridge.h
@@ -82,8 +82,8 @@ TRACE_EVENT(fdb_delete,
TP_fast_assign(
__assign_str(br_dev, br->dev->name);
__assign_str(dev, f->dst ? f->dst->dev->name : "null");
- memcpy(__entry->addr, f->addr.addr, ETH_ALEN);
- __entry->vid = f->vlan_id;
+ memcpy(__entry->addr, f->key.addr.addr, ETH_ALEN);
+ __entry->vid = f->key.vlan_id;
),
TP_printk("br_dev %s dev %s addr %02x:%02x:%02x:%02x:%02x:%02x vid %u",
diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index af5b8c87f590..1285ca30ab0a 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -125,9 +125,16 @@ static int br_dev_init(struct net_device *dev)
if (!br->stats)
return -ENOMEM;
+ err = br_fdb_hash_init(br);
+ if (err) {
+ free_percpu(br->stats);
+ return err;
+ }
+
err = br_vlan_init(br);
if (err) {
free_percpu(br->stats);
+ br_fdb_hash_fini(br);
return err;
}
@@ -135,6 +142,7 @@ static int br_dev_init(struct net_device *dev)
if (err) {
free_percpu(br->stats);
br_vlan_flush(br);
+ br_fdb_hash_fini(br);
}
br_set_lockdep_class(dev);
@@ -148,6 +156,7 @@ static void br_dev_uninit(struct net_device *dev)
br_multicast_dev_del(br);
br_multicast_uninit_stats(br);
br_vlan_flush(br);
+ br_fdb_hash_fini(br);
free_percpu(br->stats);
}
@@ -416,6 +425,7 @@ void br_dev_setup(struct net_device *dev)
br->dev = dev;
spin_lock_init(&br->lock);
INIT_LIST_HEAD(&br->port_list);
+ INIT_HLIST_HEAD(&br->fdb_list);
spin_lock_init(&br->hash_lock);
br->bridge_id.prio[0] = 0x80;
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index 4ea5c8bbe286..dc87fbc9a23b 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -28,14 +28,20 @@
#include <trace/events/bridge.h>
#include "br_private.h"
+static const struct rhashtable_params br_fdb_rht_params = {
+ .head_offset = offsetof(struct net_bridge_fdb_entry, rhnode),
+ .key_offset = offsetof(struct net_bridge_fdb_entry, key),
+ .key_len = sizeof(struct net_bridge_fdb_key),
+ .automatic_shrinking = true,
+ .locks_mul = 1,
+};
+
static struct kmem_cache *br_fdb_cache __read_mostly;
static int fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
const unsigned char *addr, u16 vid);
static void fdb_notify(struct net_bridge *br,
const struct net_bridge_fdb_entry *, int);
-static u32 fdb_salt __read_mostly;
-
int __init br_fdb_init(void)
{
br_fdb_cache = kmem_cache_create("bridge_fdb_cache",
@@ -45,7 +51,6 @@ int __init br_fdb_init(void)
if (!br_fdb_cache)
return -ENOMEM;
- get_random_bytes(&fdb_salt, sizeof(fdb_salt));
return 0;
}
@@ -54,6 +59,15 @@ void br_fdb_fini(void)
kmem_cache_destroy(br_fdb_cache);
}
+int br_fdb_hash_init(struct net_bridge *br)
+{
+ return rhashtable_init(&br->fdb_hash_tbl, &br_fdb_rht_params);
+}
+
+void br_fdb_hash_fini(struct net_bridge *br)
+{
+ rhashtable_destroy(&br->fdb_hash_tbl);
+}
/* if topology_changing then use forward_delay (default 15 sec)
* otherwise keep longer (default 5 minutes)
@@ -70,13 +84,6 @@ static inline int has_expired(const struct net_bridge *br,
time_before_eq(fdb->updated + hold_time(br), jiffies);
}
-static inline int br_mac_hash(const unsigned char *mac, __u16 vid)
-{
- /* use 1 byte of OUI and 3 bytes of NIC */
- u32 key = get_unaligned((u32 *)(mac + 2));
- return jhash_2words(key, vid, fdb_salt) & (BR_HASH_SIZE - 1);
-}
-
static void fdb_rcu_free(struct rcu_head *head)
{
struct net_bridge_fdb_entry *ent
@@ -84,19 +91,18 @@ static void fdb_rcu_free(struct rcu_head *head)
kmem_cache_free(br_fdb_cache, ent);
}
-static struct net_bridge_fdb_entry *fdb_find_rcu(struct hlist_head *head,
+static struct net_bridge_fdb_entry *fdb_find_rcu(struct rhashtable *tbl,
const unsigned char *addr,
__u16 vid)
{
- struct net_bridge_fdb_entry *f;
+ struct net_bridge_fdb_key key;
WARN_ON_ONCE(!rcu_read_lock_held());
- hlist_for_each_entry_rcu(f, head, hlist)
- if (ether_addr_equal(f->addr.addr, addr) && f->vlan_id == vid)
- break;
+ key.vlan_id = vid;
+ memcpy(key.addr.addr, addr, sizeof(key.addr.addr));
- return f;
+ return rhashtable_lookup(tbl, &key, br_fdb_rht_params);
}
/* requires bridge hash_lock */
@@ -104,13 +110,12 @@ static struct net_bridge_fdb_entry *br_fdb_find(struct net_bridge *br,
const unsigned char *addr,
__u16 vid)
{
- struct hlist_head *head = &br->hash[br_mac_hash(addr, vid)];
struct net_bridge_fdb_entry *fdb;
lockdep_assert_held_once(&br->hash_lock);
rcu_read_lock();
- fdb = fdb_find_rcu(head, addr, vid);
+ fdb = fdb_find_rcu(&br->fdb_hash_tbl, addr, vid);
rcu_read_unlock();
return fdb;
@@ -120,9 +125,7 @@ struct net_bridge_fdb_entry *br_fdb_find_rcu(struct net_bridge *br,
const unsigned char *addr,
__u16 vid)
{
- struct hlist_head *head = &br->hash[br_mac_hash(addr, vid)];
-
- return fdb_find_rcu(head, addr, vid);
+ return fdb_find_rcu(&br->fdb_hash_tbl, addr, vid);
}
/* When a static FDB entry is added, the mac address from the entry is
@@ -175,9 +178,11 @@ static void fdb_delete(struct net_bridge *br, struct net_bridge_fdb_entry *f)
trace_fdb_delete(br, f);
if (f->is_static)
- fdb_del_hw_addr(br, f->addr.addr);
+ fdb_del_hw_addr(br, f->key.addr.addr);
- hlist_del_init_rcu(&f->hlist);
+ hlist_del_init_rcu(&f->fdb_node);
+ rhashtable_remove_fast(&br->fdb_hash_tbl, &f->rhnode,
+ br_fdb_rht_params);
fdb_notify(br, f, RTM_DELNEIGH);
call_rcu(&f->rcu, fdb_rcu_free);
}
@@ -187,11 +192,11 @@ static void fdb_delete_local(struct net_bridge *br,
const struct net_bridge_port *p,
struct net_bridge_fdb_entry *f)
{
- const unsigned char *addr = f->addr.addr;
+ const unsigned char *addr = f->key.addr.addr;
struct net_bridge_vlan_group *vg;
const struct net_bridge_vlan *v;
struct net_bridge_port *op;
- u16 vid = f->vlan_id;
+ u16 vid = f->key.vlan_id;
/* Maybe another port has same hw addr? */
list_for_each_entry(op, &br->port_list, list) {
@@ -233,31 +238,23 @@ void br_fdb_find_delete_local(struct net_bridge *br,
void br_fdb_changeaddr(struct net_bridge_port *p, const unsigned char *newaddr)
{
struct net_bridge_vlan_group *vg;
+ struct net_bridge_fdb_entry *f;
struct net_bridge *br = p->br;
struct net_bridge_vlan *v;
- int i;
spin_lock_bh(&br->hash_lock);
-
vg = nbp_vlan_group(p);
- /* Search all chains since old address/hash is unknown */
- for (i = 0; i < BR_HASH_SIZE; i++) {
- struct hlist_node *h;
- hlist_for_each(h, &br->hash[i]) {
- struct net_bridge_fdb_entry *f;
-
- f = hlist_entry(h, struct net_bridge_fdb_entry, hlist);
- if (f->dst == p && f->is_local && !f->added_by_user) {
- /* delete old one */
- fdb_delete_local(br, p, f);
-
- /* if this port has no vlan information
- * configured, we can safely be done at
- * this point.
- */
- if (!vg || !vg->num_vlans)
- goto insert;
- }
+ hlist_for_each_entry(f, &br->fdb_list, fdb_node) {
+ if (f->dst == p && f->is_local && !f->added_by_user) {
+ /* delete old one */
+ fdb_delete_local(br, p, f);
+
+ /* if this port has no vlan information
+ * configured, we can safely be done at
+ * this point.
+ */
+ if (!vg || !vg->num_vlans)
+ goto insert;
}
}
@@ -316,35 +313,32 @@ void br_fdb_cleanup(struct work_struct *work)
{
struct net_bridge *br = container_of(work, struct net_bridge,
gc_work.work);
+ struct net_bridge_fdb_entry *f = NULL;
unsigned long delay = hold_time(br);
unsigned long work_delay = delay;
unsigned long now = jiffies;
- int i;
- for (i = 0; i < BR_HASH_SIZE; i++) {
- struct net_bridge_fdb_entry *f;
- struct hlist_node *n;
+ /* this part is tricky, in order to avoid blocking learning and
+ * consequently forwarding, we rely on rcu to delete objects with
+ * delayed freeing allowing us to continue traversing
+ */
+ rcu_read_lock();
+ hlist_for_each_entry_rcu(f, &br->fdb_list, fdb_node) {
+ unsigned long this_timer;
- if (!br->hash[i].first)
+ if (f->is_static || f->added_by_external_learn)
continue;
-
- spin_lock_bh(&br->hash_lock);
- hlist_for_each_entry_safe(f, n, &br->hash[i], hlist) {
- unsigned long this_timer;
-
- if (f->is_static)
- continue;
- if (f->added_by_external_learn)
- continue;
- this_timer = f->updated + delay;
- if (time_after(this_timer, now))
- work_delay = min(work_delay, this_timer - now);
- else
+ this_timer = f->updated + delay;
+ if (time_after(this_timer, now)) {
+ work_delay = min(work_delay, this_timer - now);
+ } else {
+ spin_lock_bh(&br->hash_lock);
+ if (!hlist_unhashed(&f->fdb_node))
fdb_delete(br, f);
+ spin_unlock_bh(&br->hash_lock);
}
- spin_unlock_bh(&br->hash_lock);
- cond_resched();
}
+ rcu_read_unlock();
/* Cleanup minimum 10 milliseconds apart */
work_delay = max_t(unsigned long, work_delay, msecs_to_jiffies(10));
@@ -354,16 +348,13 @@ void br_fdb_cleanup(struct work_struct *work)
/* Completely flush all dynamic entries in forwarding database.*/
void br_fdb_flush(struct net_bridge *br)
{
- int i;
+ struct net_bridge_fdb_entry *f;
+ struct hlist_node *tmp;
spin_lock_bh(&br->hash_lock);
- for (i = 0; i < BR_HASH_SIZE; i++) {
- struct net_bridge_fdb_entry *f;
- struct hlist_node *n;
- hlist_for_each_entry_safe(f, n, &br->hash[i], hlist) {
- if (!f->is_static)
- fdb_delete(br, f);
- }
+ hlist_for_each_entry_safe(f, tmp, &br->fdb_list, fdb_node) {
+ if (!f->is_static)
+ fdb_delete(br, f);
}
spin_unlock_bh(&br->hash_lock);
}
@@ -377,27 +368,22 @@ void br_fdb_delete_by_port(struct net_bridge *br,
u16 vid,
int do_all)
{
- int i;
+ struct net_bridge_fdb_entry *f;
+ struct hlist_node *tmp;
spin_lock_bh(&br->hash_lock);
- for (i = 0; i < BR_HASH_SIZE; i++) {
- struct hlist_node *h, *g;
+ hlist_for_each_entry_safe(f, tmp, &br->fdb_list, fdb_node) {
+ if (f->dst != p)
+ continue;
- hlist_for_each_safe(h, g, &br->hash[i]) {
- struct net_bridge_fdb_entry *f
- = hlist_entry(h, struct net_bridge_fdb_entry, hlist);
- if (f->dst != p)
+ if (!do_all)
+ if (f->is_static || (vid && f->key.vlan_id != vid))
continue;
- if (!do_all)
- if (f->is_static || (vid && f->vlan_id != vid))
- continue;
-
- if (f->is_local)
- fdb_delete_local(br, p, f);
- else
- fdb_delete(br, f);
- }
+ if (f->is_local)
+ fdb_delete_local(br, p, f);
+ else
+ fdb_delete(br, f);
}
spin_unlock_bh(&br->hash_lock);
}
@@ -433,52 +419,48 @@ int br_fdb_test_addr(struct net_device *dev, unsigned char *addr)
int br_fdb_fillbuf(struct net_bridge *br, void *buf,
unsigned long maxnum, unsigned long skip)
{
- struct __fdb_entry *fe = buf;
- int i, num = 0;
struct net_bridge_fdb_entry *f;
+ struct __fdb_entry *fe = buf;
+ int num = 0;
memset(buf, 0, maxnum*sizeof(struct __fdb_entry));
rcu_read_lock();
- for (i = 0; i < BR_HASH_SIZE; i++) {
- hlist_for_each_entry_rcu(f, &br->hash[i], hlist) {
- if (num >= maxnum)
- goto out;
+ hlist_for_each_entry_rcu(f, &br->fdb_list, fdb_node) {
+ if (num >= maxnum)
+ break;
- if (has_expired(br, f))
- continue;
+ if (has_expired(br, f))
+ continue;
- /* ignore pseudo entry for local MAC address */
- if (!f->dst)
- continue;
+ /* ignore pseudo entry for local MAC address */
+ if (!f->dst)
+ continue;
- if (skip) {
- --skip;
- continue;
- }
+ if (skip) {
+ --skip;
+ continue;
+ }
- /* convert from internal format to API */
- memcpy(fe->mac_addr, f->addr.addr, ETH_ALEN);
+ /* convert from internal format to API */
+ memcpy(fe->mac_addr, f->key.addr.addr, ETH_ALEN);
- /* due to ABI compat need to split into hi/lo */
- fe->port_no = f->dst->port_no;
- fe->port_hi = f->dst->port_no >> 8;
+ /* due to ABI compat need to split into hi/lo */
+ fe->port_no = f->dst->port_no;
+ fe->port_hi = f->dst->port_no >> 8;
- fe->is_local = f->is_local;
- if (!f->is_static)
- fe->ageing_timer_value = jiffies_delta_to_clock_t(jiffies - f->updated);
- ++fe;
- ++num;
- }
+ fe->is_local = f->is_local;
+ if (!f->is_static)
+ fe->ageing_timer_value = jiffies_delta_to_clock_t(jiffies - f->updated);
+ ++fe;
+ ++num;
}
-
- out:
rcu_read_unlock();
return num;
}
-static struct net_bridge_fdb_entry *fdb_create(struct hlist_head *head,
+static struct net_bridge_fdb_entry *fdb_create(struct net_bridge *br,
struct net_bridge_port *source,
const unsigned char *addr,
__u16 vid,
@@ -489,16 +471,23 @@ static struct net_bridge_fdb_entry *fdb_create(struct hlist_head *head,
fdb = kmem_cache_alloc(br_fdb_cache, GFP_ATOMIC);
if (fdb) {
- memcpy(fdb->addr.addr, addr, ETH_ALEN);
+ memcpy(fdb->key.addr.addr, addr, ETH_ALEN);
fdb->dst = source;
- fdb->vlan_id = vid;
+ fdb->key.vlan_id = vid;
fdb->is_local = is_local;
fdb->is_static = is_static;
fdb->added_by_user = 0;
fdb->added_by_external_learn = 0;
fdb->offloaded = 0;
fdb->updated = fdb->used = jiffies;
- hlist_add_head_rcu(&fdb->hlist, head);
+ if (rhashtable_lookup_insert_fast(&br->fdb_hash_tbl,
+ &fdb->rhnode,
+ br_fdb_rht_params)) {
+ kmem_cache_free(br_fdb_cache, fdb);
+ fdb = NULL;
+ } else {
+ hlist_add_head_rcu(&fdb->fdb_node, &br->fdb_list);
+ }
}
return fdb;
}
@@ -506,7 +495,6 @@ static struct net_bridge_fdb_entry *fdb_create(struct hlist_head *head,
static int fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
const unsigned char *addr, u16 vid)
{
- struct hlist_head *head = &br->hash[br_mac_hash(addr, vid)];
struct net_bridge_fdb_entry *fdb;
if (!is_valid_ether_addr(addr))
@@ -524,7 +512,7 @@ static int fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
fdb_delete(br, fdb);
}
- fdb = fdb_create(head, source, addr, vid, 1, 1);
+ fdb = fdb_create(br, source, addr, vid, 1, 1);
if (!fdb)
return -ENOMEM;
@@ -548,7 +536,6 @@ int br_fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
const unsigned char *addr, u16 vid, bool added_by_user)
{
- struct hlist_head *head = &br->hash[br_mac_hash(addr, vid)];
struct net_bridge_fdb_entry *fdb;
bool fdb_modified = false;
@@ -561,7 +548,7 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
source->state == BR_STATE_FORWARDING))
return;
- fdb = fdb_find_rcu(head, addr, vid);
+ fdb = fdb_find_rcu(&br->fdb_hash_tbl, addr, vid);
if (likely(fdb)) {
/* attempt to update an entry for a local interface */
if (unlikely(fdb->is_local)) {
@@ -590,14 +577,13 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
}
} else {
spin_lock(&br->hash_lock);
- if (likely(!fdb_find_rcu(head, addr, vid))) {
- fdb = fdb_create(head, source, addr, vid, 0, 0);
- if (fdb) {
- if (unlikely(added_by_user))
- fdb->added_by_user = 1;
- trace_br_fdb_update(br, source, addr, vid, added_by_user);
- fdb_notify(br, fdb, RTM_NEWNEIGH);
- }
+ fdb = fdb_create(br, source, addr, vid, 0, 0);
+ if (fdb) {
+ if (unlikely(added_by_user))
+ fdb->added_by_user = 1;
+ trace_br_fdb_update(br, source, addr, vid,
+ added_by_user);
+ fdb_notify(br, fdb, RTM_NEWNEIGH);
}
/* else we lose race and someone else inserts
* it first, don't bother updating
@@ -646,7 +632,7 @@ static int fdb_fill_info(struct sk_buff *skb, const struct net_bridge *br,
if (fdb->added_by_external_learn)
ndm->ndm_flags |= NTF_EXT_LEARNED;
- if (nla_put(skb, NDA_LLADDR, ETH_ALEN, &fdb->addr))
+ if (nla_put(skb, NDA_LLADDR, ETH_ALEN, &fdb->key.addr))
goto nla_put_failure;
if (nla_put_u32(skb, NDA_MASTER, br->dev->ifindex))
goto nla_put_failure;
@@ -657,7 +643,8 @@ static int fdb_fill_info(struct sk_buff *skb, const struct net_bridge *br,
if (nla_put(skb, NDA_CACHEINFO, sizeof(ci), &ci))
goto nla_put_failure;
- if (fdb->vlan_id && nla_put(skb, NDA_VLAN, sizeof(u16), &fdb->vlan_id))
+ if (fdb->key.vlan_id && nla_put(skb, NDA_VLAN, sizeof(u16),
+ &fdb->key.vlan_id))
goto nla_put_failure;
nlmsg_end(skb, nlh);
@@ -711,54 +698,48 @@ int br_fdb_dump(struct sk_buff *skb,
int *idx)
{
struct net_bridge *br = netdev_priv(dev);
+ struct net_bridge_fdb_entry *f;
int err = 0;
- int i;
if (!(dev->priv_flags & IFF_EBRIDGE))
- goto out;
+ return err;
if (!filter_dev) {
err = ndo_dflt_fdb_dump(skb, cb, dev, NULL, idx);
if (err < 0)
- goto out;
+ return err;
}
- for (i = 0; i < BR_HASH_SIZE; i++) {
- struct net_bridge_fdb_entry *f;
-
- hlist_for_each_entry_rcu(f, &br->hash[i], hlist) {
-
- if (*idx < cb->args[2])
+ rcu_read_lock();
+ hlist_for_each_entry_rcu(f, &br->fdb_list, fdb_node) {
+ if (*idx < cb->args[2])
+ goto skip;
+ if (filter_dev && (!f->dst || f->dst->dev != filter_dev)) {
+ if (filter_dev != dev)
goto skip;
-
- if (filter_dev &&
- (!f->dst || f->dst->dev != filter_dev)) {
- if (filter_dev != dev)
- goto skip;
- /* !f->dst is a special case for bridge
- * It means the MAC belongs to the bridge
- * Therefore need a little more filtering
- * we only want to dump the !f->dst case
- */
- if (f->dst)
- goto skip;
- }
- if (!filter_dev && f->dst)
+ /* !f->dst is a special case for bridge
+ * It means the MAC belongs to the bridge
+ * Therefore need a little more filtering
+ * we only want to dump the !f->dst case
+ */
+ if (f->dst)
goto skip;
-
- err = fdb_fill_info(skb, br, f,
- NETLINK_CB(cb->skb).portid,
- cb->nlh->nlmsg_seq,
- RTM_NEWNEIGH,
- NLM_F_MULTI);
- if (err < 0)
- goto out;
-skip:
- *idx += 1;
}
+ if (!filter_dev && f->dst)
+ goto skip;
+
+ err = fdb_fill_info(skb, br, f,
+ NETLINK_CB(cb->skb).portid,
+ cb->nlh->nlmsg_seq,
+ RTM_NEWNEIGH,
+ NLM_F_MULTI);
+ if (err < 0)
+ break;
+skip:
+ *idx += 1;
}
+ rcu_read_unlock();
-out:
return err;
}
@@ -766,7 +747,6 @@ int br_fdb_dump(struct sk_buff *skb,
static int fdb_add_entry(struct net_bridge *br, struct net_bridge_port *source,
const __u8 *addr, __u16 state, __u16 flags, __u16 vid)
{
- struct hlist_head *head = &br->hash[br_mac_hash(addr, vid)];
struct net_bridge_fdb_entry *fdb;
bool modified = false;
@@ -787,7 +767,7 @@ static int fdb_add_entry(struct net_bridge *br, struct net_bridge_port *source,
if (!(flags & NLM_F_CREATE))
return -ENOENT;
- fdb = fdb_create(head, source, addr, vid, 0, 0);
+ fdb = fdb_create(br, source, addr, vid, 0, 0);
if (!fdb)
return -ENOMEM;
@@ -1012,65 +992,60 @@ int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
int br_fdb_sync_static(struct net_bridge *br, struct net_bridge_port *p)
{
- struct net_bridge_fdb_entry *fdb, *tmp;
- int i;
+ struct net_bridge_fdb_entry *f, *tmp;
int err;
ASSERT_RTNL();
- for (i = 0; i < BR_HASH_SIZE; i++) {
- hlist_for_each_entry(fdb, &br->hash[i], hlist) {
- /* We only care for static entries */
- if (!fdb->is_static)
- continue;
-
- err = dev_uc_add(p->dev, fdb->addr.addr);
- if (err)
- goto rollback;
- }
+ /* the key here is that static entries change only under rtnl */
+ rcu_read_lock();
+ hlist_for_each_entry_rcu(f, &br->fdb_list, fdb_node) {
+ /* We only care for static entries */
+ if (!f->is_static)
+ continue;
+ err = dev_uc_add(p->dev, f->key.addr.addr);
+ if (err)
+ goto rollback;
}
- return 0;
+done:
+ rcu_read_unlock();
-rollback:
- for (i = 0; i < BR_HASH_SIZE; i++) {
- hlist_for_each_entry(tmp, &br->hash[i], hlist) {
- /* If we reached the fdb that failed, we can stop */
- if (tmp == fdb)
- break;
-
- /* We only care for static entries */
- if (!tmp->is_static)
- continue;
+ return err;
- dev_uc_del(p->dev, tmp->addr.addr);
- }
+rollback:
+ hlist_for_each_entry_rcu(tmp, &br->fdb_list, fdb_node) {
+ /* We only care for static entries */
+ if (!tmp->is_static)
+ continue;
+ if (tmp == f)
+ break;
+ dev_uc_del(p->dev, tmp->key.addr.addr);
}
- return err;
+
+ goto done;
}
void br_fdb_unsync_static(struct net_bridge *br, struct net_bridge_port *p)
{
- struct net_bridge_fdb_entry *fdb;
- int i;
+ struct net_bridge_fdb_entry *f;
ASSERT_RTNL();
- for (i = 0; i < BR_HASH_SIZE; i++) {
- hlist_for_each_entry_rcu(fdb, &br->hash[i], hlist) {
- /* We only care for static entries */
- if (!fdb->is_static)
- continue;
+ rcu_read_lock();
+ hlist_for_each_entry_rcu(f, &br->fdb_list, fdb_node) {
+ /* We only care for static entries */
+ if (!f->is_static)
+ continue;
- dev_uc_del(p->dev, fdb->addr.addr);
- }
+ dev_uc_del(p->dev, f->key.addr.addr);
}
+ rcu_read_unlock();
}
int br_fdb_external_learn_add(struct net_bridge *br, struct net_bridge_port *p,
const unsigned char *addr, u16 vid)
{
struct net_bridge_fdb_entry *fdb;
- struct hlist_head *head;
bool modified = false;
int err = 0;
@@ -1078,10 +1053,9 @@ int br_fdb_external_learn_add(struct net_bridge *br, struct net_bridge_port *p,
spin_lock_bh(&br->hash_lock);
- head = &br->hash[br_mac_hash(addr, vid)];
fdb = br_fdb_find(br, addr, vid);
if (!fdb) {
- fdb = fdb_create(head, p, addr, vid, 0, 0);
+ fdb = fdb_create(br, p, addr, vid, 0, 0);
if (!fdb) {
err = -ENOMEM;
goto err_unlock;
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 1312b8d20ec3..80559fd11b7e 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -168,12 +168,17 @@ struct net_bridge_vlan_group {
u16 pvid;
};
+struct net_bridge_fdb_key {
+ mac_addr addr;
+ u16 vlan_id;
+};
+
struct net_bridge_fdb_entry {
- struct hlist_node hlist;
+ struct rhash_head rhnode;
struct net_bridge_port *dst;
- mac_addr addr;
- __u16 vlan_id;
+ struct net_bridge_fdb_key key;
+ struct hlist_node fdb_node;
unsigned char is_local:1,
is_static:1,
added_by_user:1,
@@ -315,7 +320,7 @@ struct net_bridge {
struct net_bridge_vlan_group __rcu *vlgrp;
#endif
- struct hlist_head hash[BR_HASH_SIZE];
+ struct rhashtable fdb_hash_tbl;
#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
union {
struct rtable fake_rtable;
@@ -405,6 +410,7 @@ struct net_bridge {
int offload_fwd_mark;
#endif
bool neigh_suppress_enabled;
+ struct hlist_head fdb_list;
};
struct br_input_skb_cb {
@@ -515,6 +521,8 @@ static inline void br_netpoll_disable(struct net_bridge_port *p)
/* br_fdb.c */
int br_fdb_init(void);
void br_fdb_fini(void);
+int br_fdb_hash_init(struct net_bridge *br);
+void br_fdb_hash_fini(struct net_bridge *br);
void br_fdb_flush(struct net_bridge *br);
void br_fdb_find_delete_local(struct net_bridge *br,
const struct net_bridge_port *p,
diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c
index 9700e0f3307b..ee775f4ff76c 100644
--- a/net/bridge/br_switchdev.c
+++ b/net/bridge/br_switchdev.c
@@ -121,13 +121,13 @@ br_switchdev_fdb_notify(const struct net_bridge_fdb_entry *fdb, int type)
switch (type) {
case RTM_DELNEIGH:
- br_switchdev_fdb_call_notifiers(false, fdb->addr.addr,
- fdb->vlan_id,
+ br_switchdev_fdb_call_notifiers(false, fdb->key.addr.addr,
+ fdb->key.vlan_id,
fdb->dst->dev);
break;
case RTM_NEWNEIGH:
- br_switchdev_fdb_call_notifiers(true, fdb->addr.addr,
- fdb->vlan_id,
+ br_switchdev_fdb_call_notifiers(true, fdb->key.addr.addr,
+ fdb->key.vlan_id,
fdb->dst->dev);
break;
}
--
2.1.4
^ permalink raw reply related
* Re: [PATCH net-next 4/8] sctp: implement report_ftsn for sctp_stream_interleave
From: Xin Long @ 2017-12-12 14:02 UTC (permalink / raw)
To: Marcelo Ricardo Leitner; +Cc: network dev, linux-sctp, Neil Horman, davem
In-Reply-To: <20171212132514.GC3532@localhost.localdomain>
On Tue, Dec 12, 2017 at 9:25 PM, Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
> On Tue, Dec 12, 2017 at 05:25:55PM +0800, Xin Long wrote:
>> report_ftsn is added as a member of sctp_stream_interleave, used to
>> skip tsn from tsnmap, remove old events from reasm or lobby queue,
>> and abort pd for data or idata, called for SCTP_CMD_REPORT_FWDTSN
>> cmd and asoc reset.
>>
>> sctp_report_iftsn works for ifwdtsn, and sctp_report_fwdtsn works
>> for fwdtsn. Note that sctp_report_iftsn doesn't do asoc abort_pd,
>> as stream abort_pd will be done when handling ifwdtsn. But when
>> ftsn is equal with ftsn, which means asoc reset, asoc abot_pd has
> nit... aboRt ^^^^ :-)
ah sorry, 'set spell' could tell when it's together with _pd. :D
^ permalink raw reply
* Re: [PATCH net-next 3/8] sctp: implement validate_ftsn for sctp_stream_interleave
From: Xin Long @ 2017-12-12 13:59 UTC (permalink / raw)
To: Marcelo Ricardo Leitner; +Cc: network dev, linux-sctp, Neil Horman, davem
In-Reply-To: <20171212133700.GD3532@localhost.localdomain>
On Tue, Dec 12, 2017 at 9:37 PM, Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
> On Tue, Dec 12, 2017 at 09:31:36PM +0800, Xin Long wrote:
>> On Tue, Dec 12, 2017 at 9:18 PM, Marcelo Ricardo Leitner
>> <marcelo.leitner@gmail.com> wrote:
>> > On Tue, Dec 12, 2017 at 05:25:54PM +0800, Xin Long wrote:
>> > ...
>> >> --- a/net/sctp/sm_statetable.c
>> >> +++ b/net/sctp/sm_statetable.c
>> >> @@ -992,7 +992,8 @@ static const struct sctp_sm_table_entry *sctp_chunk_event_lookup(
>> >> return &chunk_event_table[cid][state];
>> >>
>> >> if (net->sctp.prsctp_enable) {
>> >> - if (cid == SCTP_CID_FWD_TSN)
>> >> + if (cid == SCTP_CID_FWD_TSN ||
>> >> + (net->sctp.intl_enable && cid == SCTP_CID_I_FWD_TSN))
>> >
>> > We don't really need to check intl_enable here, do we?
>> We think net->sctp.xxx_enable as a main switch here,
>> that means it only accepts SCTP_CID_I_FWD_TSN
>> when this switch is open.
>
> Yes, but
>
>>
>> As for the check in validate_ftsn(), it follows the RFC checking
>> on asoc's intl_enable.
>
> Yes.
>
>>
>> does it make sense from this perspective ?
>
> Sort of. We need consistency. If we want do discard such invalid
> chunks right here, that condition also needs to ensure that fwd_tsn
> chunks are actually allowed (IOW, that intl_enable is false).
>
> And we don't do any check for idata chunks. We just just accept them
> and discard later when trying to consume it.
Yeah, seems not a good idea to check intl_enable here, it's not like
other _enable.
intl_enable is more like a 'which' option, while other _enables are
like 'yes or no' option.
I will just remove intl_enable checks, for both ifwdtsn and idata.
Thanks.
>
>>
>>
>>
>> > Because a) it should actually look like:
>> > - if (cid == SCTP_CID_FWD_TSN)
>> > + if ((!net->sctp.intl_enable && cid == SCTP_CID_FWD_TSN) ||
>> > + (net->sctp.intl_enable && cid == SCTP_CID_I_FWD_TSN))
>> > but b) we will validate the chunk format/feature later with
>> > validate_ftsn(), similarly to what happens with data chunks, so it
>> > seems the check on intl_enable here is not necessary.
>> >
>> > Same happens with data chunks, btw.
>> >
>> >> return &prsctp_chunk_event_table[0][state];
>> >> }
>> >>
>> >> diff --git a/net/sctp/stream_interleave.c b/net/sctp/stream_interleave.c
>> >> index 2ead372..cc4a5e3 100644
>> >> --- a/net/sctp/stream_interleave.c
>> >> +++ b/net/sctp/stream_interleave.c
>> >> @@ -1153,8 +1153,49 @@ static void sctp_generate_iftsn(struct sctp_outq *q, __u32 ctsn)
>> >> }
>> >> }
>> >>
>> >> +#define _sctp_walk_ifwdtsn(pos, chunk, end) \
>> >> + for (pos = chunk->subh.ifwdtsn_hdr->skip; \
>> >> + (void *)pos < (void *)chunk->subh.ifwdtsn_hdr->skip + (end); pos++)
>> >> +
>> >> +#define sctp_walk_ifwdtsn(pos, ch) \
>> >> + _sctp_walk_ifwdtsn((pos), (ch), ntohs((ch)->chunk_hdr->length) - \
>> >> + sizeof(struct sctp_ifwdtsn_chunk))
>> >> +
>> >> +static bool sctp_validate_fwdtsn(struct sctp_chunk *chunk)
>> >> +{
>> >> + struct sctp_fwdtsn_skip *skip;
>> >> + __u16 incnt;
>> >> +
>> >> + if (chunk->chunk_hdr->type != SCTP_CID_FWD_TSN)
>> >> + return false;
>> >> +
>> >> + incnt = chunk->asoc->stream.incnt;
>> >> + sctp_walk_fwdtsn(skip, chunk)
>> >> + if (ntohs(skip->stream) >= incnt)
>> >> + return false;
>> >> +
>> >> + return true;
>> >> +}
>> >> +
>> >> +static bool sctp_validate_iftsn(struct sctp_chunk *chunk)
>> >> +{
>> >> + struct sctp_ifwdtsn_skip *skip;
>> >> + __u16 incnt;
>> >> +
>> >> + if (chunk->chunk_hdr->type != SCTP_CID_I_FWD_TSN)
>> >> + return false;
>> >> +
>> >> + incnt = chunk->asoc->stream.incnt;
>> >> + sctp_walk_ifwdtsn(skip, chunk)
>> >> + if (ntohs(skip->stream) >= incnt)
>> >> + return false;
>> >> +
>> >> + return true;
>> >> +}
>> >> +
>> >> static struct sctp_stream_interleave sctp_stream_interleave_0 = {
>> >> .data_chunk_len = sizeof(struct sctp_data_chunk),
>> >> + .ftsn_chunk_len = sizeof(struct sctp_fwdtsn_chunk),
>> >> /* DATA process functions */
>> >> .make_datafrag = sctp_make_datafrag_empty,
>> >> .assign_number = sctp_chunk_assign_ssn,
>> >> @@ -1166,10 +1207,12 @@ static struct sctp_stream_interleave sctp_stream_interleave_0 = {
>> >> .abort_pd = sctp_ulpq_abort_pd,
>> >> /* FORWARD-TSN process functions */
>> >> .generate_ftsn = sctp_generate_fwdtsn,
>> >> + .validate_ftsn = sctp_validate_fwdtsn,
>> >> };
>> >>
>> >> static struct sctp_stream_interleave sctp_stream_interleave_1 = {
>> >> .data_chunk_len = sizeof(struct sctp_idata_chunk),
>> >> + .ftsn_chunk_len = sizeof(struct sctp_ifwdtsn_chunk),
>> >> /* I-DATA process functions */
>> >> .make_datafrag = sctp_make_idatafrag_empty,
>> >> .assign_number = sctp_chunk_assign_mid,
>> >> @@ -1181,6 +1224,7 @@ static struct sctp_stream_interleave sctp_stream_interleave_1 = {
>> >> .abort_pd = sctp_intl_abort_pd,
>> >> /* I-FORWARD-TSN process functions */
>> >> .generate_ftsn = sctp_generate_iftsn,
>> >> + .validate_ftsn = sctp_validate_iftsn,
>> >> };
>> >>
>> >> void sctp_stream_interleave_init(struct sctp_stream *stream)
>> >> --
>> >> 2.1.0
>> >>
>> >> --
>> >> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
>> >> the body of a message to majordomo@vger.kernel.org
>> >> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> >>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
^ permalink raw reply
* [PATCH net-next v2] ip6_vti: adjust vti mtu according to mtu of output device
From: Alexey Kodanev @ 2017-12-12 13:53 UTC (permalink / raw)
To: netdev
Cc: Steffen Klassert, David Miller, Petr Vorel, Shannon Nelson,
Alexey Kodanev
LTP/udp6_ipsec_vti tests fail when sending large UDP datagrams that
require fragmentation and the underlying device has MTU <= 1500. This
happens because ip6_vti sets mtu to ETH_DATA_LEN and not updating it
depending on a destination address or link parameter.
Further attempts to send UDP packets may succeed because pmtu gets
updated on ICMPV6_PKT_TOOBIG in vti6_err().
Here is the example when the output device MTU is set to 9000:
# ip a sh ltp_ns_veth2
ltp_ns_veth2@if7: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 9000 ...
inet 10.0.0.2/24 scope global ltp_ns_veth2
inet6 fd00::2/64 scope global
# ip li add vti6 type vti6 local fd00::2 remote fd00::1
# ip li show vti6
vti6@NONE: <POINTOPOINT,NOARP> mtu 1500 ...
link/tunnel6 fd00::2 peer fd00::1
After the patch:
# ip li add vti6 type vti6 local fd00::2 remote fd00::1
# ip li show vti6
vti6@NONE: <POINTOPOINT,NOARP> mtu 8832 ...
link/tunnel6 fd00::2 peer fd00::1
Regarding ip_vti, it already tunes MTU with ip_tunnel_bind_dev().
Reported-by: Petr Vorel <pvorel@suse.cz>
Signed-off-by: Alexey Kodanev <alexey.kodanev@oracle.com>
---
v2: * cleanup commit message issues (thanks to Shannon)
* handle the case when we don't have route but have device parameter
* cast new MTU to int and then check the maximum (tdev->mtu can be
less than dev->hard_header_len)
When changing the tunnel parameters, MTU can be updated as well... should
we also check that parms 'link', 'laddr' or 'raddr' were actually changed
in vti6_tnl_change() and/or IFLA_MTU wasn't set?
net/ipv6/ip6_vti.c | 22 ++++++++++++++++++++++
1 files changed, 22 insertions(+), 0 deletions(-)
diff --git a/net/ipv6/ip6_vti.c b/net/ipv6/ip6_vti.c
index dbb74f3..d4624c2 100644
--- a/net/ipv6/ip6_vti.c
+++ b/net/ipv6/ip6_vti.c
@@ -626,6 +626,7 @@ static void vti6_link_config(struct ip6_tnl *t)
{
struct net_device *dev = t->dev;
struct __ip6_tnl_parm *p = &t->parms;
+ struct net_device *tdev = NULL;
memcpy(dev->dev_addr, &p->laddr, sizeof(struct in6_addr));
memcpy(dev->broadcast, &p->raddr, sizeof(struct in6_addr));
@@ -638,6 +639,27 @@ static void vti6_link_config(struct ip6_tnl *t)
dev->flags |= IFF_POINTOPOINT;
else
dev->flags &= ~IFF_POINTOPOINT;
+
+ if (p->flags & IP6_TNL_F_CAP_XMIT) {
+ int strict = (ipv6_addr_type(&p->raddr) &
+ (IPV6_ADDR_MULTICAST | IPV6_ADDR_LINKLOCAL));
+
+ struct rt6_info *rt = rt6_lookup(t->net,
+ &p->raddr, &p->laddr,
+ p->link, strict);
+
+ if (rt)
+ tdev = rt->dst.dev;
+ ip6_rt_put(rt);
+ }
+
+ if (!tdev && p->link)
+ tdev = __dev_get_by_index(t->net, p->link);
+
+ if (tdev) {
+ dev->mtu = max_t(int, tdev->mtu - dev->hard_header_len,
+ IPV6_MIN_MTU);
+ }
}
/**
--
1.7.1
^ permalink raw reply related
* Re: [PATCH net-next 3/8] sctp: implement validate_ftsn for sctp_stream_interleave
From: Marcelo Ricardo Leitner @ 2017-12-12 13:37 UTC (permalink / raw)
To: Xin Long; +Cc: network dev, linux-sctp, Neil Horman, davem
In-Reply-To: <CADvbK_d5ZqBtyuWYOJ19gwcd51g0Vgcdq-xCjevqwGpTgC-yDQ@mail.gmail.com>
On Tue, Dec 12, 2017 at 09:31:36PM +0800, Xin Long wrote:
> On Tue, Dec 12, 2017 at 9:18 PM, Marcelo Ricardo Leitner
> <marcelo.leitner@gmail.com> wrote:
> > On Tue, Dec 12, 2017 at 05:25:54PM +0800, Xin Long wrote:
> > ...
> >> --- a/net/sctp/sm_statetable.c
> >> +++ b/net/sctp/sm_statetable.c
> >> @@ -992,7 +992,8 @@ static const struct sctp_sm_table_entry *sctp_chunk_event_lookup(
> >> return &chunk_event_table[cid][state];
> >>
> >> if (net->sctp.prsctp_enable) {
> >> - if (cid == SCTP_CID_FWD_TSN)
> >> + if (cid == SCTP_CID_FWD_TSN ||
> >> + (net->sctp.intl_enable && cid == SCTP_CID_I_FWD_TSN))
> >
> > We don't really need to check intl_enable here, do we?
> We think net->sctp.xxx_enable as a main switch here,
> that means it only accepts SCTP_CID_I_FWD_TSN
> when this switch is open.
Yes, but
>
> As for the check in validate_ftsn(), it follows the RFC checking
> on asoc's intl_enable.
Yes.
>
> does it make sense from this perspective ?
Sort of. We need consistency. If we want do discard such invalid
chunks right here, that condition also needs to ensure that fwd_tsn
chunks are actually allowed (IOW, that intl_enable is false).
And we don't do any check for idata chunks. We just just accept them
and discard later when trying to consume it.
>
>
>
> > Because a) it should actually look like:
> > - if (cid == SCTP_CID_FWD_TSN)
> > + if ((!net->sctp.intl_enable && cid == SCTP_CID_FWD_TSN) ||
> > + (net->sctp.intl_enable && cid == SCTP_CID_I_FWD_TSN))
> > but b) we will validate the chunk format/feature later with
> > validate_ftsn(), similarly to what happens with data chunks, so it
> > seems the check on intl_enable here is not necessary.
> >
> > Same happens with data chunks, btw.
> >
> >> return &prsctp_chunk_event_table[0][state];
> >> }
> >>
> >> diff --git a/net/sctp/stream_interleave.c b/net/sctp/stream_interleave.c
> >> index 2ead372..cc4a5e3 100644
> >> --- a/net/sctp/stream_interleave.c
> >> +++ b/net/sctp/stream_interleave.c
> >> @@ -1153,8 +1153,49 @@ static void sctp_generate_iftsn(struct sctp_outq *q, __u32 ctsn)
> >> }
> >> }
> >>
> >> +#define _sctp_walk_ifwdtsn(pos, chunk, end) \
> >> + for (pos = chunk->subh.ifwdtsn_hdr->skip; \
> >> + (void *)pos < (void *)chunk->subh.ifwdtsn_hdr->skip + (end); pos++)
> >> +
> >> +#define sctp_walk_ifwdtsn(pos, ch) \
> >> + _sctp_walk_ifwdtsn((pos), (ch), ntohs((ch)->chunk_hdr->length) - \
> >> + sizeof(struct sctp_ifwdtsn_chunk))
> >> +
> >> +static bool sctp_validate_fwdtsn(struct sctp_chunk *chunk)
> >> +{
> >> + struct sctp_fwdtsn_skip *skip;
> >> + __u16 incnt;
> >> +
> >> + if (chunk->chunk_hdr->type != SCTP_CID_FWD_TSN)
> >> + return false;
> >> +
> >> + incnt = chunk->asoc->stream.incnt;
> >> + sctp_walk_fwdtsn(skip, chunk)
> >> + if (ntohs(skip->stream) >= incnt)
> >> + return false;
> >> +
> >> + return true;
> >> +}
> >> +
> >> +static bool sctp_validate_iftsn(struct sctp_chunk *chunk)
> >> +{
> >> + struct sctp_ifwdtsn_skip *skip;
> >> + __u16 incnt;
> >> +
> >> + if (chunk->chunk_hdr->type != SCTP_CID_I_FWD_TSN)
> >> + return false;
> >> +
> >> + incnt = chunk->asoc->stream.incnt;
> >> + sctp_walk_ifwdtsn(skip, chunk)
> >> + if (ntohs(skip->stream) >= incnt)
> >> + return false;
> >> +
> >> + return true;
> >> +}
> >> +
> >> static struct sctp_stream_interleave sctp_stream_interleave_0 = {
> >> .data_chunk_len = sizeof(struct sctp_data_chunk),
> >> + .ftsn_chunk_len = sizeof(struct sctp_fwdtsn_chunk),
> >> /* DATA process functions */
> >> .make_datafrag = sctp_make_datafrag_empty,
> >> .assign_number = sctp_chunk_assign_ssn,
> >> @@ -1166,10 +1207,12 @@ static struct sctp_stream_interleave sctp_stream_interleave_0 = {
> >> .abort_pd = sctp_ulpq_abort_pd,
> >> /* FORWARD-TSN process functions */
> >> .generate_ftsn = sctp_generate_fwdtsn,
> >> + .validate_ftsn = sctp_validate_fwdtsn,
> >> };
> >>
> >> static struct sctp_stream_interleave sctp_stream_interleave_1 = {
> >> .data_chunk_len = sizeof(struct sctp_idata_chunk),
> >> + .ftsn_chunk_len = sizeof(struct sctp_ifwdtsn_chunk),
> >> /* I-DATA process functions */
> >> .make_datafrag = sctp_make_idatafrag_empty,
> >> .assign_number = sctp_chunk_assign_mid,
> >> @@ -1181,6 +1224,7 @@ static struct sctp_stream_interleave sctp_stream_interleave_1 = {
> >> .abort_pd = sctp_intl_abort_pd,
> >> /* I-FORWARD-TSN process functions */
> >> .generate_ftsn = sctp_generate_iftsn,
> >> + .validate_ftsn = sctp_validate_iftsn,
> >> };
> >>
> >> void sctp_stream_interleave_init(struct sctp_stream *stream)
> >> --
> >> 2.1.0
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> >> the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at http://vger.kernel.org/majordomo-info.html
> >>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [BUG] skge: a possible sleep-in-atomic bug in skge_remove
From: David Miller @ 2017-12-12 13:34 UTC (permalink / raw)
To: baijiaju1990
Cc: mlindner, stephen, shemminger, shemminger, netdev, linux-kernel
In-Reply-To: <1e8a8196-f0d1-2a82-3632-b882787c4391@gmail.com>
From: Jia-Ju Bai <baijiaju1990@gmail.com>
Date: Tue, 12 Dec 2017 16:38:12 +0800
> According to drivers/net/ethernet/marvell/skge.c, the driver may sleep
> under a spinlock.
> The function call path is:
> skge_remove (acquire the spinlock)
> free_irq --> may sleep
>
> I do not find a good way to fix it, so I only report.
> This possible bug is found by my static analysis tool (DSAC) and
> checked by my code review.
This was added by:
commit a9e9fd7182332d0cf5f3e601df3e71dd431b70d7
Author: Stephen Hemminger <shemminger@vyatta.com>
Date: Tue Sep 27 13:41:37 2011 -0400
skge: handle irq better on single port card
I think the free_irq() can be moved below the unlock.
Stephen, please take a look.
Thanks!
^ permalink raw reply
* Re: [PATCH net-next 3/8] sctp: implement validate_ftsn for sctp_stream_interleave
From: Xin Long @ 2017-12-12 13:31 UTC (permalink / raw)
To: Marcelo Ricardo Leitner; +Cc: network dev, linux-sctp, Neil Horman, davem
In-Reply-To: <20171212131853.GB3532@localhost.localdomain>
On Tue, Dec 12, 2017 at 9:18 PM, Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
> On Tue, Dec 12, 2017 at 05:25:54PM +0800, Xin Long wrote:
> ...
>> --- a/net/sctp/sm_statetable.c
>> +++ b/net/sctp/sm_statetable.c
>> @@ -992,7 +992,8 @@ static const struct sctp_sm_table_entry *sctp_chunk_event_lookup(
>> return &chunk_event_table[cid][state];
>>
>> if (net->sctp.prsctp_enable) {
>> - if (cid == SCTP_CID_FWD_TSN)
>> + if (cid == SCTP_CID_FWD_TSN ||
>> + (net->sctp.intl_enable && cid == SCTP_CID_I_FWD_TSN))
>
> We don't really need to check intl_enable here, do we?
We think net->sctp.xxx_enable as a main switch here,
that means it only accepts SCTP_CID_I_FWD_TSN
when this switch is open.
As for the check in validate_ftsn(), it follows the RFC checking
on asoc's intl_enable.
does it make sense from this perspective ?
> Because a) it should actually look like:
> - if (cid == SCTP_CID_FWD_TSN)
> + if ((!net->sctp.intl_enable && cid == SCTP_CID_FWD_TSN) ||
> + (net->sctp.intl_enable && cid == SCTP_CID_I_FWD_TSN))
> but b) we will validate the chunk format/feature later with
> validate_ftsn(), similarly to what happens with data chunks, so it
> seems the check on intl_enable here is not necessary.
>
> Same happens with data chunks, btw.
>
>> return &prsctp_chunk_event_table[0][state];
>> }
>>
>> diff --git a/net/sctp/stream_interleave.c b/net/sctp/stream_interleave.c
>> index 2ead372..cc4a5e3 100644
>> --- a/net/sctp/stream_interleave.c
>> +++ b/net/sctp/stream_interleave.c
>> @@ -1153,8 +1153,49 @@ static void sctp_generate_iftsn(struct sctp_outq *q, __u32 ctsn)
>> }
>> }
>>
>> +#define _sctp_walk_ifwdtsn(pos, chunk, end) \
>> + for (pos = chunk->subh.ifwdtsn_hdr->skip; \
>> + (void *)pos < (void *)chunk->subh.ifwdtsn_hdr->skip + (end); pos++)
>> +
>> +#define sctp_walk_ifwdtsn(pos, ch) \
>> + _sctp_walk_ifwdtsn((pos), (ch), ntohs((ch)->chunk_hdr->length) - \
>> + sizeof(struct sctp_ifwdtsn_chunk))
>> +
>> +static bool sctp_validate_fwdtsn(struct sctp_chunk *chunk)
>> +{
>> + struct sctp_fwdtsn_skip *skip;
>> + __u16 incnt;
>> +
>> + if (chunk->chunk_hdr->type != SCTP_CID_FWD_TSN)
>> + return false;
>> +
>> + incnt = chunk->asoc->stream.incnt;
>> + sctp_walk_fwdtsn(skip, chunk)
>> + if (ntohs(skip->stream) >= incnt)
>> + return false;
>> +
>> + return true;
>> +}
>> +
>> +static bool sctp_validate_iftsn(struct sctp_chunk *chunk)
>> +{
>> + struct sctp_ifwdtsn_skip *skip;
>> + __u16 incnt;
>> +
>> + if (chunk->chunk_hdr->type != SCTP_CID_I_FWD_TSN)
>> + return false;
>> +
>> + incnt = chunk->asoc->stream.incnt;
>> + sctp_walk_ifwdtsn(skip, chunk)
>> + if (ntohs(skip->stream) >= incnt)
>> + return false;
>> +
>> + return true;
>> +}
>> +
>> static struct sctp_stream_interleave sctp_stream_interleave_0 = {
>> .data_chunk_len = sizeof(struct sctp_data_chunk),
>> + .ftsn_chunk_len = sizeof(struct sctp_fwdtsn_chunk),
>> /* DATA process functions */
>> .make_datafrag = sctp_make_datafrag_empty,
>> .assign_number = sctp_chunk_assign_ssn,
>> @@ -1166,10 +1207,12 @@ static struct sctp_stream_interleave sctp_stream_interleave_0 = {
>> .abort_pd = sctp_ulpq_abort_pd,
>> /* FORWARD-TSN process functions */
>> .generate_ftsn = sctp_generate_fwdtsn,
>> + .validate_ftsn = sctp_validate_fwdtsn,
>> };
>>
>> static struct sctp_stream_interleave sctp_stream_interleave_1 = {
>> .data_chunk_len = sizeof(struct sctp_idata_chunk),
>> + .ftsn_chunk_len = sizeof(struct sctp_ifwdtsn_chunk),
>> /* I-DATA process functions */
>> .make_datafrag = sctp_make_idatafrag_empty,
>> .assign_number = sctp_chunk_assign_mid,
>> @@ -1181,6 +1224,7 @@ static struct sctp_stream_interleave sctp_stream_interleave_1 = {
>> .abort_pd = sctp_intl_abort_pd,
>> /* I-FORWARD-TSN process functions */
>> .generate_ftsn = sctp_generate_iftsn,
>> + .validate_ftsn = sctp_validate_iftsn,
>> };
>>
>> void sctp_stream_interleave_init(struct sctp_stream *stream)
>> --
>> 2.1.0
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
^ 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