* Re: [bpf PATCH 1/2] bpf: sockmap, error path can not release psock in multi-map case
From: Daniel Borkmann @ 2018-07-03 14:40 UTC (permalink / raw)
To: John Fastabend, ast; +Cc: netdev, kafai
In-Reply-To: <20180630135131.6395.86522.stgit@john-Precision-Tower-5810>
On 06/30/2018 03:51 PM, John Fastabend wrote:
> The current code, in the error path of sock_hash_ctx_update_elem,
> checks if the sock has a psock in the user data and if so decrements
> the reference count of the psock. However, if the error happens early
> in the error path we may have never incremented the psock reference
> count and if the psock exists because the sock is in another map then
> we may inadvertently decrement the reference count.
>
> Fix this by making the error path only call smap_release_sock if the
> error happens after the increment.
>
> Reported-by: syzbot+d464d2c20c717ef5a6a8@syzkaller.appspotmail.com
> Fixes: 81110384441a ("bpf: sockmap, add hash map support")
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
> ---
> 0 files changed
>
> diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
> index 4fc2cb1..63fb047 100644
> --- a/kernel/bpf/sockmap.c
> +++ b/kernel/bpf/sockmap.c
> @@ -1896,7 +1896,7 @@ static int __sock_map_ctx_update_elem(struct bpf_map *map,
> e = kzalloc(sizeof(*e), GFP_ATOMIC | __GFP_NOWARN);
> if (!e) {
> err = -ENOMEM;
> - goto out_progs;
> + goto out_free;
> }
> }
>
> @@ -2324,7 +2324,12 @@ static int sock_hash_ctx_update_elem(struct bpf_sock_ops_kern *skops,
> if (err)
> goto err;
>
> - /* bpf_map_update_elem() can be called in_irq() */
> + psock = smap_psock_sk(sock);
> + if (unlikely(!psock)) {
> + err = -EINVAL;
> + goto err;
> + }
Is an error even possible at this point? If __sock_map_ctx_update_elem() succeeds,
we either allocated and linked a new psock to the sock or we inc'ed the existing
one's refcount. From my reading it seems we should always succeed the subsequent
smap_psock_sk(). If we would have failed here in between it would mean we'd have
a refcount imbalance somewhere?
> +
> raw_spin_lock_bh(&b->lock);
> l_old = lookup_elem_raw(head, hash, key, key_size);
> if (l_old && map_flags == BPF_NOEXIST) {
> @@ -2342,12 +2347,6 @@ static int sock_hash_ctx_update_elem(struct bpf_sock_ops_kern *skops,
> goto bucket_err;
> }
>
> - psock = smap_psock_sk(sock);
> - if (unlikely(!psock)) {
> - err = -EINVAL;
> - goto bucket_err;
> - }
> -
> rcu_assign_pointer(e->hash_link, l_new);
> rcu_assign_pointer(e->htab,
> container_of(map, struct bpf_htab, map));
> @@ -2370,12 +2369,10 @@ static int sock_hash_ctx_update_elem(struct bpf_sock_ops_kern *skops,
> raw_spin_unlock_bh(&b->lock);
> return 0;
> bucket_err:
> + smap_release_sock(psock, sock);
> raw_spin_unlock_bh(&b->lock);
> err:
> kfree(e);
> - psock = smap_psock_sk(sock);
> - if (psock)
> - smap_release_sock(psock, sock);
> return err;
> }
>
>
Thanks,
Daniel
^ permalink raw reply
* Re: pull-request: wireless-drivers 2018-07-03
From: David Miller @ 2018-07-03 14:29 UTC (permalink / raw)
To: kvalo; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <87tvpgr3n4.fsf@kamboji.qca.qualcomm.com>
From: Kalle Valo <kvalo@codeaurora.org>
Date: Tue, 03 Jul 2018 13:54:07 +0300
> this is the first pull request to net tree for 4.18, more info below and
> please let me know if you have any problems.
Pulled, thanks Kalle.
^ permalink raw reply
* Re: [PATCH] staging: fsl-dpaa2/ethsw: Update maintainers for Ethernet Switch driver
From: Greg KH @ 2018-07-03 14:29 UTC (permalink / raw)
To: Razvan Stefanescu; +Cc: devel, netdev, linux-kernel, ioana.ciornei
In-Reply-To: <1530626724-20833-1-git-send-email-razvan.stefanescu@nxp.com>
On Tue, Jul 03, 2018 at 05:05:24PM +0300, Razvan Stefanescu wrote:
> Signed-off-by: Razvan Stefanescu <razvan.stefanescu@nxp.com>
No changelog text?
> ---
> MAINTAINERS | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b6d0cc0..0d36546 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4434,7 +4434,8 @@ S: Maintained
> F: drivers/staging/fsl-dpaa2/ethernet
>
> DPAA2 ETHERNET SWITCH DRIVER
> -M: Razvan Stefanescu <razvan.stefanescu@nxp.com>
> +M: Ioana Radulescu <ruxandra.radulescu@nxp.com>
> +M: Ioana Ciornei <ioana.ciornei@nxp.com>
No ack/signed-off-by from either of these developers?
I can't take this as-is :(
thanks,
greg k-h
^ permalink raw reply
* [PATCH net] smsc75xx: Add workaround for gigabit link up hardware errata.
From: Yuiko Oshino @ 2018-07-03 15:21 UTC (permalink / raw)
To: davem; +Cc: netdev, UNGLinuxDriver
In certain conditions, the device may not be able to link in gigabit mode. This software workaround ensures that the device will not enter the failure state.
Fixes: d0cad871703b898a442e4049c532ec39168e5b57 ("SMSC75XX USB 2.0 Gigabit Ethernet Devices")
Signed-off-by: Yuiko Oshino <yuiko.oshino@microchip.com>
---
drivers/net/usb/smsc75xx.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 62 insertions(+)
diff --git a/drivers/net/usb/smsc75xx.c b/drivers/net/usb/smsc75xx.c
index 7a6a1fe..05553d2 100644
--- a/drivers/net/usb/smsc75xx.c
+++ b/drivers/net/usb/smsc75xx.c
@@ -82,6 +82,9 @@ static bool turbo_mode = true;
module_param(turbo_mode, bool, 0644);
MODULE_PARM_DESC(turbo_mode, "Enable multiple frames per Rx transaction");
+static int smsc75xx_link_ok_nopm(struct usbnet *dev);
+static int smsc75xx_phy_gig_workaround(struct usbnet *dev);
+
static int __must_check __smsc75xx_read_reg(struct usbnet *dev, u32 index,
u32 *data, int in_pm)
{
@@ -852,6 +855,9 @@ static int smsc75xx_phy_initialize(struct usbnet *dev)
return -EIO;
}
+ /* phy workaround for gig link */
+ smsc75xx_phy_gig_workaround(dev);
+
smsc75xx_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE,
ADVERTISE_ALL | ADVERTISE_CSMA | ADVERTISE_PAUSE_CAP |
ADVERTISE_PAUSE_ASYM);
@@ -987,6 +993,62 @@ static int smsc75xx_wait_ready(struct usbnet *dev, int in_pm)
return -EIO;
}
+static int smsc75xx_phy_gig_workaround(struct usbnet *dev)
+{
+ struct mii_if_info *mii = &dev->mii;
+ int ret = 0, timeout = 0;
+ u32 buf, link_up = 0;
+
+ /* Set the phy in Gig loopback */
+ smsc75xx_mdio_write(dev->net, mii->phy_id, MII_BMCR, 0x4040);
+
+ /* Wait for the link up */
+ do {
+ link_up = smsc75xx_link_ok_nopm(dev);
+ usleep_range(10000, 20000);
+ timeout++;
+ } while ((!link_up) && (timeout < 1000));
+
+ if (timeout >= 1000) {
+ netdev_warn(dev->net, "Timeout waiting for PHY link up\n");
+ return -EIO;
+ }
+
+ /* phy reset */
+ ret = smsc75xx_read_reg(dev, PMT_CTL, &buf);
+ if (ret < 0) {
+ netdev_warn(dev->net, "Failed to read PMT_CTL: %d\n", ret);
+ return ret;
+ }
+
+ buf |= PMT_CTL_PHY_RST;
+
+ ret = smsc75xx_write_reg(dev, PMT_CTL, buf);
+ if (ret < 0) {
+ netdev_warn(dev->net, "Failed to write PMT_CTL: %d\n", ret);
+ return ret;
+ }
+
+ timeout = 0;
+ do {
+ usleep_range(10000, 20000);
+ ret = smsc75xx_read_reg(dev, PMT_CTL, &buf);
+ if (ret < 0) {
+ netdev_warn(dev->net, "Failed to read PMT_CTL: %d\n",
+ ret);
+ return ret;
+ }
+ timeout++;
+ } while ((buf & PMT_CTL_PHY_RST) && (timeout < 100));
+
+ if (timeout >= 100) {
+ netdev_warn(dev->net, "timeout waiting for PHY Reset\n");
+ return -EIO;
+ }
+
+ return 0;
+}
+
static int smsc75xx_reset(struct usbnet *dev)
{
struct smsc75xx_priv *pdata = (struct smsc75xx_priv *)(dev->data[0]);
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net] r8169: fix mac address change
From: David Miller @ 2018-07-03 14:28 UTC (permalink / raw)
To: hkallweit1; +Cc: vinschen, netdev
In-Reply-To: <1770bc23-75f1-d462-b8a3-61e81ade4c3c@gmail.com>
From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Mon, 2 Jul 2018 22:49:35 +0200
> Network core refuses to change mac address because flag
> IFF_LIVE_ADDR_CHANGE isn't set. Set this missing flag.
>
> Fixes: 1f7aa2bc268e ("r8169: simplify rtl_set_mac_address")
> Reported-by: Corinna Vinschen <vinschen@redhat.com>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Applied and queued up for -stable.
^ permalink raw reply
* Re: [PATCH] lib: rhashtable: Correct self-assignment in rhashtable.c
From: David Miller @ 2018-07-03 14:26 UTC (permalink / raw)
To: rishabhb; +Cc: netdev, neilb, tgraf, herbert, linux-arm-msm
In-Reply-To: <1530549334-13890-1-git-send-email-rishabhb@codeaurora.org>
From: Rishabh Bhatnagar <rishabhb@codeaurora.org>
Date: Mon, 2 Jul 2018 09:35:34 -0700
> In file lib/rhashtable.c line 777, skip variable is assigned to
> itself. The following error was observed:
>
> lib/rhashtable.c:777:41: warning: explicitly assigning value of
> variable of type 'int' to itself [-Wself-assign] error, forbidden
> warning: rhashtable.c:777
> This error was found when compiling with Clang 6.0. Change it to iter->skip.
>
> Signed-off-by: Rishabh Bhatnagar <rishabhb@codeaurora.org>
> Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
> Reviewed-by: NeilBrown <neilb@suse.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next v3 0/5] net: aquantia: various ethtool ops implementation
From: David Miller @ 2018-07-03 14:24 UTC (permalink / raw)
To: igor.russkikh; +Cc: netdev, darcari, pavel.belous
In-Reply-To: <cover.1530537192.git.igor.russkikh@aquantia.com>
From: Igor Russkikh <igor.russkikh@aquantia.com>
Date: Mon, 2 Jul 2018 17:03:34 +0300
> In this patchset Anton Mikaev and I added some useful ethtool operations:
> - ring size changes
> - link renegotioation
> - flow control management
>
> The patch also improves init/deinit sequence.
>
> V3 changes:
> - After review and analysis it is clear that rtnl lock (which is
> captured by default on ethtool ops) is enough to secure possible
> overlapping of dev open/close. Thus, just dropping internal mutex.
>
> V2 changes:
> - using mutex to secure simultaneous dev close/open
> - using state var to store/restore dev state
Series applied.
^ permalink raw reply
* Re: [PATCH] net: stmmac_tc: use 64-bit arithmetic instead of 32-bit
From: David Miller @ 2018-07-03 14:21 UTC (permalink / raw)
To: gustavo; +Cc: peppe.cavallaro, alexandre.torgue, joabreu, netdev, linux-kernel
In-Reply-To: <20180702120932.GA13079@embeddedor.com>
From: "Gustavo A. R. Silva" <gustavo@embeddedor.com>
Date: Mon, 2 Jul 2018 07:09:32 -0500
> Add suffix UL to constant 1024 in order to give the compiler complete
> information about the proper arithmetic to use. Notice that this
> constant is used in a context that expects an expression of type
> u64 (64 bits, unsigned) and following expressions are currently
> being evaluated using 32-bit arithmetic:
>
> qopt->idleslope * 1024 * ptr
> qopt->hicredit * 1024 * 8
> qopt->locredit * 1024 * 8
>
> Addresses-Coverity-ID: 1470246 ("Unintentional integer overflow")
> Addresses-Coverity-ID: 1470248 ("Unintentional integer overflow")
> Addresses-Coverity-ID: 1470249 ("Unintentional integer overflow")
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
Applied.
^ permalink raw reply
* Re: [net-next,v1] tcp: Improve setsockopt() TCP_USER_TIMEOUT accuracy
From: Neal Cardwell @ 2018-07-03 14:13 UTC (permalink / raw)
To: Jonathan Maxwell
Cc: David Miller, Eric Dumazet, Alexey Kuznetsov, Hideaki YOSHIFUJI,
Netdev, LKML, jmaxwell
In-Reply-To: <20180703072113.19910-1-jmaxwell37@gmail.com>
On Tue, Jul 3, 2018 at 3:21 AM Jon Maxwell <jmaxwell37@gmail.com> wrote:
>
> v1 contains the following suggestions by Neal Cardwell:
>
> 1) Fix up units mismatch regarding msec/jiffies.
> 2) Address possiblility of time_remaining being negative.
> 3) Add a helper routine to do the rto calculation.
>
> Every time the TCP retransmission timer fires. It checks to see if there is a
> timeout before scheduling the next retransmit timer. The retransmit interval
> between each retransmission increases exponentially. The issue is that in order
> for the timeout to occur the retransmit timer needs to fire again. If the user
> timeout check happens after the 9th retransmit for example. It needs to wait for
> the 10th retransmit timer to fire in order to evaluate whether a timeout has
> occurred or not. If the interval is large enough then the timeout will be
> inaccurate.
>
> For example with a TCP_USER_TIMEOUT of 10 seconds without patch:
>
> 1st retransmit:
>
> 22:25:18.973488 IP host1.49310 > host2.search-agent: Flags [.]
>
> Last retransmit:
>
> 22:25:26.205499 IP host1.49310 > host2.search-agent: Flags [.]
>
> Timeout:
>
> send: Connection timed out
> Sun Jul 1 22:25:34 EDT 2018
>
> We can see that last retransmit took ~7 seconds. Which pushed the total
> timeout to ~15 seconds instead of the expected 10 seconds. This gets more
> inaccurate the larger the TCP_USER_TIMEOUT value. As the interval increases.
>
> Add tcp_clamp_rto_to_user_timeout() to determine if the user rto has expired.
> Or whether the rto interval needs to be recalculated. Use the original interval
> if user rto is not set.
>
> Test results with the patch is the expected 10 second timeout:
>
> 1st retransmit:
>
> 01:37:59.022555 IP host1.49310 > host2.search-agent: Flags [.]
>
> Last retransmit:
>
> 01:38:06.486558 IP host1.49310 > host2.search-agent: Flags [.]
>
> Timeout:
>
> send: Connection timed out
> Mon Jul 2 01:38:09 EDT 2018
>
> Signed-off-by: Jon Maxwell <jmaxwell37@gmail.com>
> ---
> net/ipv4/tcp_timer.c | 21 ++++++++++++++++++++-
> 1 file changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
> index 3b3611729928..82c2a3b3713c 100644
> --- a/net/ipv4/tcp_timer.c
> +++ b/net/ipv4/tcp_timer.c
> @@ -22,6 +22,23 @@
> #include <linux/gfp.h>
> #include <net/tcp.h>
>
> +static __u32 tcp_clamp_rto_to_user_timeout(struct sock *sk)
> +{
> + struct inet_connection_sock *icsk = inet_csk(sk);
> + __u32 rto = icsk->icsk_rto;
> + __u32 elapsed, user_timeout;
> +
> + if (!icsk->icsk_user_timeout)
> + return rto;
> + elapsed = tcp_time_stamp(tcp_sk(sk)) - tcp_sk(sk)->retrans_stamp;
Thanks. The local logic seems OK to me now, but from reading
retransmits_timed_out() it looks like at this point in the code we are
not guaranteed that tcp_sk(sk)->retrans_stamp is initialized to
something non-zero. So we probably need a preceding preparatory patch
that factors out the first few lines of retransmits_timed_out() into
a helper frunction to get the start_ts for use in this calculation.
Perhaps:
u32 tcp_retrans_stamp():
start_ts = tcp_sk(sk)->retrans_stamp;
if (unlikely(!start_ts)) {
head = tcp_rtx_queue_head(sk);
if (!head)
return 0;
start_ts = tcp_skb_timestamp(head);
}
return start_ts;
And then the new tcp_clamp_rto_to_user_timeout() can use the helper:
...
retrans_stamp = tcp_retransmit_stamp(sk);
if (!retrans_stamp)
return rto;
elapsed = tcp_time_stamp(tcp_sk(sk)) - retrans_stamp;
...
Eric wrote those lines to recalculate start_ts, so we may want to wait
until Eric returns to review this before merging the resulting patch
series.
neal
^ permalink raw reply
* [PATCH] staging: fsl-dpaa2/ethsw: Update maintainers for Ethernet Switch driver
From: Razvan Stefanescu @ 2018-07-03 14:05 UTC (permalink / raw)
To: gregkh; +Cc: devel, linux-kernel, netdev, ruxandra.radulescu, ioana.ciornei
Signed-off-by: Razvan Stefanescu <razvan.stefanescu@nxp.com>
---
MAINTAINERS | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index b6d0cc0..0d36546 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4434,7 +4434,8 @@ S: Maintained
F: drivers/staging/fsl-dpaa2/ethernet
DPAA2 ETHERNET SWITCH DRIVER
-M: Razvan Stefanescu <razvan.stefanescu@nxp.com>
+M: Ioana Radulescu <ruxandra.radulescu@nxp.com>
+M: Ioana Ciornei <ioana.ciornei@nxp.com>
L: linux-kernel@vger.kernel.org
S: Maintained
F: drivers/staging/fsl-dpaa2/ethsw
--
1.9.1
^ permalink raw reply related
* Re: [PATCH v2 net-next 1/3] rds: Changing IP address internal representation to struct in6_addr
From: Ka-Cheong Poon @ 2018-07-03 14:02 UTC (permalink / raw)
To: David Miller; +Cc: netdev, santosh.shilimkar, rds-devel
In-Reply-To: <20180630.175049.1607480149466051192.davem@davemloft.net>
On 06/30/2018 04:50 PM, David Miller wrote:
> From: Ka-Cheong Poon <ka-cheong.poon@oracle.com>
> Date: Wed, 27 Jun 2018 03:23:27 -0700
>
>> This patch changes the internal representation of an IP address to use
>> struct in6_addr. IPv4 address is stored as an IPv4 mapped address.
>> All the functions which take an IP address as argument are also
>> changed to use struct in6_addr. But RDS socket layer is not modified
>> such that it still does not accept IPv6 address from an application.
>> And RDS layer does not accept nor initiate IPv6 connections.
>>
>> v2: Fixed sparse warnings.
>>
>> Signed-off-by: Ka-Cheong Poon <ka-cheong.poon@oracle.com>
>
> I really don't like this.
>
> An ipv4 mapped ipv6 address is not the same as an ipv4 address.
>
> You are effectively preventing the use of ipv6 connections
> using ipv4 mapped addresses.
Could you please clarify what is meant by an IPv6
connections using IPv4 mapped address? An IPv6 packet
cannot use an IPv4 mapped address as source or destination
address. Do you mean an app uses an IPv4 mapped address
in a struct sockaddr_in6 to set up an IPv4 connection?
Please note that this patch is patch #1. This patch
alone does not support RDS/IPv6. Hence this patch has
checks to prevent an app to use IPv6 address. Those
checks will be removed in patch #2.
> Also, assuming the sockaddr type based upon size is wrong.
> You have to check the family field, then you can decide
> to interpret the rest of the sockaddr in one way or another
> and also validate it's length.
>
--
K. Poon
ka-cheong.poon@oracle.com
^ permalink raw reply
* Re: [GIT] Networking
From: Ursula Braun @ 2018-07-03 13:52 UTC (permalink / raw)
To: Linus Torvalds, David Miller
Cc: Andrew Morton, Network Development, Linux Kernel Mailing List
In-Reply-To: <CA+55aFw4+YPQUh9RTJBzxdvJ9Xf_s4soPLbCXsXuhHT0TjPtZw@mail.gmail.com>
On 07/02/2018 08:29 PM, Linus Torvalds wrote:
> On Mon, Jul 2, 2018 at 6:03 AM David Miller <davem@davemloft.net> wrote:
>>
>> are available in the Git repository at:
>>
>> gitolite@ra.kernel.org:/pub/scm/linux/kernel/git/davem/net.git
>
> So the af_smc poll fixes clashed with the revert of the commit that
> caused those fixed to be done in the first place.
>
> See the changes to net/smc/af_smc.c in my commit
>
> a11e1d432b51 ("Revert changes to convert to ->poll_mask() and aio
> IOCB_CMD_POLL")
>
> vs Ursula's
>
> 24ac3a08e658 ("net/smc: rebuild nonblocking connect")
>
> I (briefly) considered just dropping Ursula's changes entirely, but
> they looked like a nice cleanup on their own, so what I did instead
> was to try to fix up my revert instead.
>
> That involved removing the release_sock/lock_sock pair around the
> ->poll() call, and removing the special "sock_poll_wait()" that got
> re-introduced by my revert, but that Ursula's changes seem to obviate.
>
> However, while I can look at the code and say "my merge makes sense to
> me", (a) I can't test it, (b) I don't actually know the rules for SMC
> sockets in the first place, and (c) I may be just incompetent.
>
> So Ursula - mind checking and testing the end result? I _think_ it's
> fine and the merge looked pretty obvious, but maybe af_smc got broken
> again.
>
Thanks Linus for your revert. I run a test, and found it is almost fine.
Just these 2 lines are missing:
---
net/smc/af_smc.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index e017b6a4452b..e7d7ff87499e 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -1345,6 +1345,8 @@ static __poll_t smc_poll(struct file *file, struct socket *sock,
if (sk->sk_err)
mask |= EPOLLERR;
} else {
+ if (sk->sk_state != SMC_CLOSED)
+ sock_poll_wait(file, sk_sleep(sk), wait);
if (sk->sk_err)
mask |= EPOLLERR;
if ((sk->sk_shutdown == SHUTDOWN_MASK) ||
--
> [ It's still going through by basic build tests, so I haven't pushed
> out my merge yet, but it should be in the usual places in a short
> while ]
>
> Thanks,
>
> Linus
>
Regards, Ursula
^ permalink raw reply related
* Re: [PATCH] 6lowpan: iphc: reset mac_header after decompress to fix panic
From: Alexander Aring @ 2018-07-03 13:48 UTC (permalink / raw)
To: Michael Scott
Cc: Alexander Aring, Jukka Rissanen, David S. Miller, linux-bluetooth,
linux-wpan, netdev, linux-kernel
In-Reply-To: <20180619234406.8217-1-michael@opensourcefoundries.com>
On Tue, Jun 19, 2018 at 04:44:06PM -0700, Michael Scott wrote:
> After decompression of 6lowpan socket data, an IPv6 header is inserted
> before the existing socket payload. After this, we reset the
> network_header value of the skb to account for the difference in payload
> size from prior to decompression + the addition of the IPv6 header.
>
> However, we fail to reset the mac_header value.
>
> Leaving the mac_header value untouched here, can cause a calculation
> error in net/packet/af_packet.c packet_rcv() function when an
> AF_PACKET socket is opened in SOCK_RAW mode for use on a 6lowpan
> interface.
>
> On line 2088, the data pointer is moved backward by the value returned
> from skb_mac_header(). If skb->data is adjusted so that it is before
> the skb->head pointer (which can happen when an old value of mac_header
> is left in place) the kernel generates a panic in net/core/skbuff.c
> line 1717.
>
> This panic can be generated by BLE 6lowpan interfaces (such as bt0) and
> 802.15.4 interfaces (such as lowpan0) as they both use the same 6lowpan
> sources for compression and decompression.
>
> Signed-off-by: Michael Scott <michael@opensourcefoundries.com>
Acked-by: Alexander Aring <aring@mojatatu.com>
Thanks!
- Alex
^ permalink raw reply
* Re: [PATCH net] net/ipv6: Revert attempt to simplify route replace and append
From: Ido Schimmel @ 2018-07-03 13:43 UTC (permalink / raw)
To: dsahern; +Cc: netdev, sharpd, Thomas.Winter, David Ahern, petrm
In-Reply-To: <20180702220312.14590-1-dsahern@kernel.org>
On Mon, Jul 02, 2018 at 03:03:12PM -0700, dsahern@kernel.org wrote:
> From: David Ahern <dsahern@gmail.com>
>
> NetworkManager likes to manage linklocal prefix routes and does so with
> the NLM_F_APPEND flag, breaking attempts to simplify the IPv6 route
> code and by extension enable multipath routes with device only nexthops.
>
> Revert f34436a43092 and its followup
> 6eba08c3626b ("ipv6: Only emit append events for appended routes").
> Update the test cases to reflect the old behavior.
>
> Fixes: f34436a43092 ("net/ipv6: Simplify route replace and appending into multipath route")
> Signed-off-by: David Ahern <dsahern@gmail.com>
> ---
> Ido: I left 5a15a1b07c51. FIB_EVENT_ENTRY_APPEND is not generated for
> IPv6, so no harm in leaving it.
OK, but I had a follow-up series that did further changes in mlxsw
(merge commit eab9a2d5f323) to support the new behavior. You want to
revert it and squash to v2 or I'll send another revert?
Also, Petr added a multipath test that relies on IPv6 device only
nexthops. See commit 54818c4c4b937.
^ permalink raw reply
* [PATCH net-next] net: sched: act_pedit: fix possible memory leak in tcf_pedit_init()
From: Wei Yongjun @ 2018-07-03 13:45 UTC (permalink / raw)
To: Jamal Hadi Salim, Cong Wang, Jiri Pirko, Amir Vadai
Cc: Wei Yongjun, netdev, kernel-janitors
'keys_ex' is malloced by tcf_pedit_keys_ex_parse() in tcf_pedit_init()
but not all of the error handle path free it, this may cause memory
leak. This patch fix it.
Fixes: 71d0ed7079df ("net/act_pedit: Support using offset relative to the conventional network headers")
Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
net/sched/act_pedit.c | 25 ++++++++++++++++---------
1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index 55bc96b..e43aef2 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -175,32 +175,35 @@ static int tcf_pedit_init(struct net *net, struct nlattr *nla,
if (!tcf_idr_check(tn, parm->index, a, bind)) {
if (!parm->nkeys) {
NL_SET_ERR_MSG_MOD(extack, "Pedit requires keys to be passed");
- return -EINVAL;
+ ret = -EINVAL;
+ goto out_free;
}
ret = tcf_idr_create(tn, parm->index, est, a,
&act_pedit_ops, bind, false);
if (ret)
- return ret;
+ goto out_free;
p = to_pedit(*a);
keys = kmalloc(ksize, GFP_KERNEL);
if (!keys) {
tcf_idr_release(*a, bind);
- kfree(keys_ex);
- return -ENOMEM;
+ ret = -ENOMEM;
+ goto out_free;
}
ret = ACT_P_CREATED;
} else {
if (bind)
- return 0;
+ goto out_free;
tcf_idr_release(*a, bind);
- if (!ovr)
- return -EEXIST;
+ if (!ovr) {
+ ret = -EEXIST;
+ goto out_free;
+ }
p = to_pedit(*a);
if (p->tcfp_nkeys && p->tcfp_nkeys != parm->nkeys) {
keys = kmalloc(ksize, GFP_KERNEL);
if (!keys) {
- kfree(keys_ex);
- return -ENOMEM;
+ ret = -ENOMEM;
+ goto out_free;
}
}
}
@@ -222,6 +225,10 @@ static int tcf_pedit_init(struct net *net, struct nlattr *nla,
if (ret == ACT_P_CREATED)
tcf_idr_insert(tn, *a);
return ret;
+out_free:
+ kfree(keys_ex);
+ return ret;
+
}
static void tcf_pedit_cleanup(struct tc_action *a)
^ permalink raw reply related
* [PATCH] rhashtable: add restart routine in rhashtable_free_and_destroy()
From: Taehee Yoo @ 2018-07-03 13:19 UTC (permalink / raw)
To: davem, tgraf, herbert; +Cc: netdev, fw, pablo, ap420073
rhashtable_free_and_destroy() cancels re-hash deferred work
then walks and destroys elements. at this moment, some elements can be
still in future_tbl. that elements are not destroyed.
test case:
nft_rhash_destroy() calls rhashtable_free_and_destroy() to destroy
all elements of sets before destroying sets and chains.
But rhashtable_free_and_destroy() doesn't destroy elements of future_tbl.
so that splat occurred.
test script:
%cat test.nft
table ip aa {
map map1 {
type ipv4_addr : verdict;
elements = {
0 : jump a0,
1 : jump a0,
2 : jump a0,
3 : jump a0,
4 : jump a0,
5 : jump a0,
6 : jump a0,
7 : jump a0,
8 : jump a0,
9 : jump a0,
}
}
chain a0 {
}
}
flush ruleset
table ip aa {
map map1 {
type ipv4_addr : verdict;
elements = {
0 : jump a0,
1 : jump a0,
2 : jump a0,
3 : jump a0,
4 : jump a0,
5 : jump a0,
6 : jump a0,
7 : jump a0,
8 : jump a0,
9 : jump a0,
}
}
chain a0 {
}
}
flush ruleset
%while :; do nft -f test.nft; done
Splat looks like:
[ 200.795603] kernel BUG at net/netfilter/nf_tables_api.c:1363!
[ 200.806944] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN PTI
[ 200.812253] CPU: 1 PID: 1582 Comm: nft Not tainted 4.17.0+ #24
[ 200.820297] Hardware name: To be filled by O.E.M. To be filled by O.E.M./Aptio CRB, BIOS 5.6.5 07/08/2015
[ 200.830309] RIP: 0010:nf_tables_chain_destroy.isra.34+0x62/0x240 [nf_tables]
[ 200.838317] Code: 43 50 85 c0 74 26 48 8b 45 00 48 8b 4d 08 ba 54 05 00 00 48 c7 c6 60 6d 29 c0 48 c7 c7 c0 65 29 c0 4c 8b 40 08 e8 58 e5 fd f8 <0f> 0b 48 89 da 48 b8 00 00 00 00 00 fc ff
[ 200.860366] RSP: 0000:ffff880118dbf4d0 EFLAGS: 00010282
[ 200.866354] RAX: 0000000000000061 RBX: ffff88010cdeaf08 RCX: 0000000000000000
[ 200.874355] RDX: 0000000000000061 RSI: 0000000000000008 RDI: ffffed00231b7e90
[ 200.882361] RBP: ffff880118dbf4e8 R08: ffffed002373bcfb R09: ffffed002373bcfa
[ 200.890354] R10: 0000000000000000 R11: ffffed002373bcfb R12: dead000000000200
[ 200.898356] R13: dead000000000100 R14: ffffffffbb62af38 R15: dffffc0000000000
[ 200.906354] FS: 00007fefc31fd700(0000) GS:ffff88011b800000(0000) knlGS:0000000000000000
[ 200.915533] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 200.922355] CR2: 0000557f1c8e9128 CR3: 0000000106880000 CR4: 00000000001006e0
[ 200.930353] Call Trace:
[ 200.932351] ? nf_tables_commit+0x26f6/0x2c60 [nf_tables]
[ 200.939525] ? nf_tables_setelem_notify.constprop.49+0x1a0/0x1a0 [nf_tables]
[ 200.947525] ? nf_tables_delchain+0x6e0/0x6e0 [nf_tables]
[ 200.952383] ? nft_add_set_elem+0x1700/0x1700 [nf_tables]
[ 200.959532] ? nla_parse+0xab/0x230
[ 200.963529] ? nfnetlink_rcv_batch+0xd06/0x10d0 [nfnetlink]
[ 200.968384] ? nfnetlink_net_init+0x130/0x130 [nfnetlink]
[ 200.975525] ? debug_show_all_locks+0x290/0x290
[ 200.980363] ? debug_show_all_locks+0x290/0x290
[ 200.986356] ? sched_clock_cpu+0x132/0x170
[ 200.990352] ? find_held_lock+0x39/0x1b0
[ 200.994355] ? sched_clock_local+0x10d/0x130
[ 200.999531] ? memset+0x1f/0x40
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
---
lib/rhashtable.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index 0e04947..8ea27fa 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -1134,6 +1134,7 @@ void rhashtable_free_and_destroy(struct rhashtable *ht,
mutex_lock(&ht->mutex);
tbl = rht_dereference(ht->tbl, ht);
if (free_fn) {
+restart:
for (i = 0; i < tbl->size; i++) {
struct rhash_head *pos, *next;
@@ -1147,9 +1148,11 @@ void rhashtable_free_and_destroy(struct rhashtable *ht,
rht_dereference(pos->next, ht) : NULL)
rhashtable_free_one(ht, pos, free_fn, arg);
}
+ tbl = rht_dereference(tbl->future_tbl, ht);
+ if (tbl)
+ goto restart;
}
-
- bucket_table_free(tbl);
+ bucket_table_free(rht_dereference(ht->tbl, ht));
mutex_unlock(&ht->mutex);
}
EXPORT_SYMBOL_GPL(rhashtable_free_and_destroy);
--
2.9.3
^ permalink raw reply related
* Re: [PATCH net-next v2 1/2] tcp: notify when a delayed ack is sent
From: Neal Cardwell @ 2018-07-03 13:14 UTC (permalink / raw)
To: Yuchung Cheng
Cc: Lawrence Brakmo, Netdev, Kernel Team, bmatheny, ast, Steve Ibanez,
Eric Dumazet
In-Reply-To: <CAK6E8=f2n9PKwne5COvT0sFyFymfJDDeE9Vxk6PeTpOEwg7z8w@mail.gmail.com>
On Mon, Jul 2, 2018 at 7:49 PM Yuchung Cheng <ycheng@google.com> wrote:
>
> On Mon, Jul 2, 2018 at 2:39 PM, Lawrence Brakmo <brakmo@fb.com> wrote:
> >
> > DCTCP depends on the CA_EVENT_NON_DELAYED_ACK and CA_EVENT_DELAYED_ACK
> > notifications to keep track if it needs to send an ACK for packets that
> > were received with a particular ECN state but whose ACK was delayed.
> >
> > Under some circumstances, for example when a delayed ACK is sent with a
> > data packet, DCTCP state was not being updated due to a lack of
> > notification that the previously delayed ACK was sent. As a result, it
> > would sometimes send a duplicate ACK when a new data packet arrived.
> >
> > This patch insures that DCTCP's state is correctly updated so it will
> > not send the duplicate ACK.
> Sorry to chime-in late here (lame excuse: IETF deadline)
>
> IIRC this issue would exist prior to 4.11 kernel. While it'd be good
> to fix that, it's not clear which patch introduced the regression
> between 4.11 and 4.16? I assume you tested Eric's most recent quickack
> fix.
I don't think Larry is saying there's a regression between 4.11 and
4.16. His recent "tcp: force cwnd at least 2 in tcp_cwnd_reduction"
patch here:
https://patchwork.ozlabs.org/patch/935233/
said that 4.11 was bad (high tail latency and lots of RTOs) and 4.16
was still bad but with different netstat counters (no RTOs but still
high tail latency):
"""
On 4.11, pcap traces indicate that in some instances the 1st packet of
the RPC is received but no ACK is sent before the packet is
retransmitted. On 4.11 netstat shows TCP timeouts, with some of them
spurious.
On 4.16, we don't see retransmits in netstat but the high tail latencies
are still there.
"""
I suspect the RTOs disappeared but latencies remained too high because
between 4.11 and 4.16 we introduced:
tcp: allow TLP in ECN CWR
https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/commit/?id=b4f70c3d4ec32a2ff4c62e1e2da0da5f55fe12bd
So the RTOs probably disappeared because this commit turned them into
TLPs. But the latencies remained high because the fundamental bug
remained throughout 4.11 and 4.16 and today: the DCTCP use of
tcp_send_ack() with an old rcv_nxt caused delayed ACKs to be cancelled
when they should not have been.
> In terms of the fix itself, it seems odd the tcp_send_ack() call in
> DCTCP generates NON_DELAYED_ACK event to toggle DCTCP's
> delayed_ack_reserved bit. Shouldn't the fix to have DCTCP send the
> "prior" ACK w/o cancelling delayed-ACK and mis-recording that it's
> cancelled, because that prior-ACK really is a supplementary old ACK.
This patch is fixing an issue that's orthogonal to the one you are
talking about. Using the taxonomy from our team's internal discussion
yesterday, the issue you mention where the DCTCP "prior" ACK is
cancelling delayed ACKs is issue (4); the issue that this particular
"tcp: notify when a delayed ack is sent" patch from Larry fixes is
issue (3). It's a bit tricky because both issues appeared in Larry's
trace summary and packetdrill script to reproduce the issue.
neal
^ permalink raw reply
* [PATCH iproute2] tc: Fix output of ip attributes
From: Roi Dayan @ 2018-07-03 12:54 UTC (permalink / raw)
To: netdev; +Cc: Jiri Pirko, Or Gerlitz, Roi Dayan
Example output is of tos and ttl.
Befoe this fix the format used %x caused output of the pointer
instead of the intended string created in the out variable.
Fixes: e28b88a464c4 ("tc: jsonify flower filter")
Signed-off-by: Roi Dayan <roid@mellanox.com>
---
tc/f_flower.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tc/f_flower.c b/tc/f_flower.c
index c710765179fb..1dfd57d286d9 100644
--- a/tc/f_flower.c
+++ b/tc/f_flower.c
@@ -1134,7 +1134,7 @@ static void flower_print_ip_attr(char *name, struct rtattr *key_attr,
if (mask_attr)
sprintf(out + done, "/%x", rta_getattr_u8(mask_attr));
- sprintf(namefrm, "\n %s %%x", name);
+ sprintf(namefrm, "\n %s %%s", name);
print_string(PRINT_ANY, name, namefrm, out);
}
--
2.7.5
^ permalink raw reply related
* [PATCH net-next 2/2] selftests: forwarding: test for bridge port isolation
From: Nikolay Aleksandrov @ 2018-07-03 12:42 UTC (permalink / raw)
To: netdev; +Cc: roopa, dsahern, idosch, stephen, Nikolay Aleksandrov
In-Reply-To: <20180703124244.6864-1-nikolay@cumulusnetworks.com>
This test checks if the bridge port isolation feature works as expected
by performing ping/ping6 tests between hosts that are isolated (should
not work) and between an isolated and non-isolated hosts (should work).
Same test is performed for flooding from and to isolated and
non-isolated ports.
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
.../net/forwarding/bridge_port_isolation.sh | 151 +++++++++++++++++++++
1 file changed, 151 insertions(+)
create mode 100755 tools/testing/selftests/net/forwarding/bridge_port_isolation.sh
diff --git a/tools/testing/selftests/net/forwarding/bridge_port_isolation.sh b/tools/testing/selftests/net/forwarding/bridge_port_isolation.sh
new file mode 100755
index 000000000000..a43b4645c4de
--- /dev/null
+++ b/tools/testing/selftests/net/forwarding/bridge_port_isolation.sh
@@ -0,0 +1,151 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+ALL_TESTS="ping_ipv4 ping_ipv6 flooding"
+NUM_NETIFS=6
+CHECK_TC="yes"
+source lib.sh
+
+h1_create()
+{
+ simple_if_init $h1 192.0.2.1/24 2001:db8:1::1/64
+}
+
+h1_destroy()
+{
+ simple_if_fini $h1 192.0.2.1/24 2001:db8:1::1/64
+}
+
+h2_create()
+{
+ simple_if_init $h2 192.0.2.2/24 2001:db8:1::2/64
+}
+
+h2_destroy()
+{
+ simple_if_fini $h2 192.0.2.2/24 2001:db8:1::2/64
+}
+
+h3_create()
+{
+ simple_if_init $h3 192.0.2.3/24 2001:db8:1::3/64
+}
+
+h3_destroy()
+{
+ simple_if_fini $h3 192.0.2.3/24 2001:db8:1::3/64
+}
+
+switch_create()
+{
+ ip link add dev br0 type bridge
+
+ ip link set dev $swp1 master br0
+ ip link set dev $swp2 master br0
+ ip link set dev $swp3 master br0
+
+ ip link set dev $swp1 type bridge_slave isolated on
+ check_err $? "Can't set isolation on port $swp1"
+ ip link set dev $swp2 type bridge_slave isolated on
+ check_err $? "Can't set isolation on port $swp2"
+ ip link set dev $swp3 type bridge_slave isolated off
+ check_err $? "Can't disable isolation on port $swp3"
+
+ ip link set dev br0 up
+ ip link set dev $swp1 up
+ ip link set dev $swp2 up
+ ip link set dev $swp3 up
+}
+
+switch_destroy()
+{
+ ip link set dev $swp3 down
+ ip link set dev $swp2 down
+ ip link set dev $swp1 down
+
+ ip link del dev br0
+}
+
+setup_prepare()
+{
+ h1=${NETIFS[p1]}
+ swp1=${NETIFS[p2]}
+
+ swp2=${NETIFS[p3]}
+ h2=${NETIFS[p4]}
+
+ swp3=${NETIFS[p5]}
+ h3=${NETIFS[p6]}
+
+ vrf_prepare
+
+ h1_create
+ h2_create
+ h3_create
+
+ switch_create
+}
+
+cleanup()
+{
+ pre_cleanup
+
+ switch_destroy
+
+ h3_destroy
+ h2_destroy
+ h1_destroy
+
+ vrf_cleanup
+}
+
+ping_ipv4()
+{
+ RET=0
+ ping_do $h1 192.0.2.2
+ check_fail $? "Ping worked when it should not have"
+
+ RET=0
+ ping_do $h3 192.0.2.2
+ check_err $? "Ping didn't work when it should have"
+
+ log_test "Isolated port ping"
+}
+
+ping_ipv6()
+{
+ RET=0
+ ping6_do $h1 2001:db8:1::2
+ check_fail $? "Ping6 worked when it should not have"
+
+ RET=0
+ ping6_do $h3 2001:db8:1::2
+ check_err $? "Ping6 didn't work when it should have"
+
+ log_test "Isolated port ping6"
+}
+
+flooding()
+{
+ local mac=de:ad:be:ef:13:37
+ local ip=192.0.2.100
+
+ RET=0
+ flood_test_do false $mac $ip $h1 $h2
+ check_err $? "Packet was flooded when it should not have been"
+
+ RET=0
+ flood_test_do true $mac $ip $h3 $h2
+ check_err $? "Packet was not flooded when it should have been"
+
+ log_test "Isolated port flooding"
+}
+
+trap cleanup EXIT
+
+setup_prepare
+setup_wait
+
+tests_run
+
+exit $EXIT_STATUS
--
2.11.0
^ permalink raw reply related
* [PATCH net-next 1/2] selftests: forwarding: lib: extract ping and ping6 so they can be reused
From: Nikolay Aleksandrov @ 2018-07-03 12:42 UTC (permalink / raw)
To: netdev; +Cc: roopa, dsahern, idosch, stephen, Nikolay Aleksandrov
In-Reply-To: <20180703124244.6864-1-nikolay@cumulusnetworks.com>
Extract ping and ping6 command execution so the return value can be
checked by the caller, this is needed for port isolation tests that are
intended to fail.
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
tools/testing/selftests/net/forwarding/lib.sh | 22 ++++++++++++++++------
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
index e073918bbe15..2bb9cf303c53 100644
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -659,30 +659,40 @@ multipath_eval()
##############################################################################
# Tests
-ping_test()
+ping_do()
{
local if_name=$1
local dip=$2
local vrf_name
- RET=0
-
vrf_name=$(master_name_get $if_name)
ip vrf exec $vrf_name $PING $dip -c 10 -i 0.1 -w 2 &> /dev/null
+}
+
+ping_test()
+{
+ RET=0
+
+ ping_do $1 $2
check_err $?
log_test "ping"
}
-ping6_test()
+ping6_do()
{
local if_name=$1
local dip=$2
local vrf_name
- RET=0
-
vrf_name=$(master_name_get $if_name)
ip vrf exec $vrf_name $PING6 $dip -c 10 -i 0.1 -w 2 &> /dev/null
+}
+
+ping6_test()
+{
+ RET=0
+
+ ping6_do $1 $2
check_err $?
log_test "ping6"
}
--
2.11.0
^ permalink raw reply related
* [PATCH iproute2 net-next] bridge: add support for isolated option
From: Nikolay Aleksandrov @ 2018-07-03 12:42 UTC (permalink / raw)
To: netdev; +Cc: roopa, dsahern, idosch, stephen, Nikolay Aleksandrov
In-Reply-To: <20180703124244.6864-1-nikolay@cumulusnetworks.com>
This patch adds support for the new isolated port option which, if set,
would allow the isolated ports to communicate only with non-isolated
ports and the bridge device. The option can be set via the bridge or ip
link type bridge_slave commands, e.g.:
$ ip link set dev eth0 type bridge_slave isolated on
$ bridge link set dev eth0 isolated on
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
bridge/link.c | 11 +++++++++++
ip/iplink_bridge_slave.c | 9 +++++++++
man/man8/bridge.8 | 6 ++++++
man/man8/ip-link.8.in | 6 ++++--
4 files changed, 30 insertions(+), 2 deletions(-)
diff --git a/bridge/link.c b/bridge/link.c
index 8d89aca2e638..9656ca338782 100644
--- a/bridge/link.c
+++ b/bridge/link.c
@@ -152,6 +152,9 @@ static void print_protinfo(FILE *fp, struct rtattr *attr)
if (prtb[IFLA_BRPORT_VLAN_TUNNEL])
print_onoff(fp, "vlan_tunnel",
rta_getattr_u8(prtb[IFLA_BRPORT_VLAN_TUNNEL]));
+ if (prtb[IFLA_BRPORT_ISOLATED])
+ print_onoff(fp, "isolated",
+ rta_getattr_u8(prtb[IFLA_BRPORT_ISOLATED]));
} else
print_portstate(rta_getattr_u8(attr));
}
@@ -250,6 +253,7 @@ static void usage(void)
fprintf(stderr, " [ mcast_flood {on | off} ]\n");
fprintf(stderr, " [ neigh_suppress {on | off} ]\n");
fprintf(stderr, " [ vlan_tunnel {on | off} ]\n");
+ fprintf(stderr, " [ isolated {on | off} ]\n");
fprintf(stderr, " [ hwmode {vepa | veb} ]\n");
fprintf(stderr, " [ self ] [ master ]\n");
fprintf(stderr, " bridge link show [dev DEV]\n");
@@ -291,6 +295,7 @@ static int brlink_modify(int argc, char **argv)
__s8 flood = -1;
__s8 vlan_tunnel = -1;
__s8 mcast_flood = -1;
+ __s8 isolated = -1;
__s8 hairpin = -1;
__s8 bpdu_guard = -1;
__s8 fast_leave = -1;
@@ -386,6 +391,10 @@ static int brlink_modify(int argc, char **argv)
if (!on_off("vlan_tunnel", &vlan_tunnel,
*argv))
return -1;
+ } else if (strcmp(*argv, "isolated") == 0) {
+ NEXT_ARG();
+ if (!on_off("isolated", &isolated, *argv))
+ return -1;
} else {
usage();
}
@@ -444,6 +453,8 @@ static int brlink_modify(int argc, char **argv)
if (vlan_tunnel != -1)
addattr8(&req.n, sizeof(req), IFLA_BRPORT_VLAN_TUNNEL,
vlan_tunnel);
+ if (isolated != -1)
+ addattr8(&req.n, sizeof(req), IFLA_BRPORT_ISOLATED, isolated);
addattr_nest_end(&req.n, nest);
diff --git a/ip/iplink_bridge_slave.c b/ip/iplink_bridge_slave.c
index 3fbfb878cdc4..5a6e48559781 100644
--- a/ip/iplink_bridge_slave.c
+++ b/ip/iplink_bridge_slave.c
@@ -40,6 +40,7 @@ static void print_explain(FILE *f)
" [ group_fwd_mask MASK ]\n"
" [ neigh_suppress {on | off} ]\n"
" [ vlan_tunnel {on | off} ]\n"
+ " [ isolated {on | off} ]\n"
);
}
@@ -274,6 +275,10 @@ static void bridge_slave_print_opt(struct link_util *lu, FILE *f,
if (tb[IFLA_BRPORT_VLAN_TUNNEL])
_print_onoff(f, "vlan_tunnel", "vlan_tunnel",
rta_getattr_u8(tb[IFLA_BRPORT_VLAN_TUNNEL]));
+
+ if (tb[IFLA_BRPORT_ISOLATED])
+ _print_onoff(f, "isolated", "isolated",
+ rta_getattr_u8(tb[IFLA_BRPORT_ISOLATED]));
}
static void bridge_slave_parse_on_off(char *arg_name, char *arg_val,
@@ -379,6 +384,10 @@ static int bridge_slave_parse_opt(struct link_util *lu, int argc, char **argv,
NEXT_ARG();
bridge_slave_parse_on_off("vlan_tunnel", *argv, n,
IFLA_BRPORT_VLAN_TUNNEL);
+ } else if (matches(*argv, "isolated") == 0) {
+ NEXT_ARG();
+ bridge_slave_parse_on_off("isolated", *argv, n,
+ IFLA_BRPORT_ISOLATED);
} else if (matches(*argv, "help") == 0) {
explain();
return -1;
diff --git a/man/man8/bridge.8 b/man/man8/bridge.8
index e7f7148315e1..f6d228c5ebfe 100644
--- a/man/man8/bridge.8
+++ b/man/man8/bridge.8
@@ -48,6 +48,7 @@ bridge \- show / manipulate bridge addresses and devices
.BR mcast_flood " { " on " | " off " } ] [ "
.BR neigh_suppress " { " on " | " off " } ] [ "
.BR vlan_tunnel " { " on " | " off " } ] [ "
+.BR isolated " { " on " | " off " } ] [ "
.BR self " ] [ " master " ]"
.ti -8
@@ -346,6 +347,11 @@ Controls whether neigh discovery (arp and nd) proxy and suppression is enabled o
Controls whether vlan to tunnel mapping is enabled on the port. By default this flag is off.
.TP
+.BR "isolated on " or " isolated off "
+Controls whether a given port will be isolated, which means it will be able to communicate with non-isolated ports only.
+By default this flag is off.
+
+.TP
.BI self
link setting is configured on specified physical device
diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in
index 83ef3cae54b9..48c238660347 100644
--- a/man/man8/ip-link.8.in
+++ b/man/man8/ip-link.8.in
@@ -2049,9 +2049,11 @@ the following additional arguments are supported:
] [
.BR group_fwd_mask " MASK"
] [
-.BR neigh_suppress " { " on " | " off " } ]"
+.BR neigh_suppress " { " on " | " off " }"
+] [
+.BR vlan_tunnel " { " on " | " off " }"
] [
-.BR vlan_tunnel " { " on " | " off " } ]"
+.BR isolated " { " on " | " off " } ]"
.in +8
.sp
--
2.11.0
^ permalink raw reply related
* [PATCH net-next 0/2] bridge: iproute2 isolated port and selftests
From: Nikolay Aleksandrov @ 2018-07-03 12:42 UTC (permalink / raw)
To: netdev; +Cc: roopa, dsahern, idosch, stephen, Nikolay Aleksandrov
Add support to iproute2 for port isolation config and selftests for it.
Nikolay Aleksandrov (2):
selftests: forwarding: lib: extract ping and ping6 so they can be
reused
selftests: forwarding: test for bridge port isolation
.../net/forwarding/bridge_port_isolation.sh | 151 +++++++++++++++++++++
tools/testing/selftests/net/forwarding/lib.sh | 22 ++-
2 files changed, 167 insertions(+), 6 deletions(-)
create mode 100755 tools/testing/selftests/net/forwarding/bridge_port_isolation.sh
--
2.11.0
^ permalink raw reply
* Re: [PATCH v4 net-next 7/9] net: ipv4: listified version of ip_rcv
From: Florian Westphal @ 2018-07-03 12:13 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: Edward Cree, davem, netdev
In-Reply-To: <20180703105035.edifjogwotv3gzwy@salvia>
Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Mon, Jul 02, 2018 at 04:14:12PM +0100, Edward Cree wrote:
> > Also involved adding a way to run a netfilter hook over a list of packets.
> > Rather than attempting to make netfilter know about lists (which would be
> > a major project in itself) we just let it call the regular okfn (in this
> > case ip_rcv_finish()) for any packets it steals, and have it give us back
> > a list of packets it's synchronously accepted (which normally NF_HOOK
> > would automatically call okfn() on, but we want to be able to potentially
> > pass the list to a listified version of okfn().)
> > The netfilter hooks themselves are indirect calls that still happen per-
> > packet (see nf_hook_entry_hookfn()), but again, changing that can be left
> > for future work.
> >
> > There is potential for out-of-order receives if the netfilter hook ends up
> > synchronously stealing packets, as they will be processed before any
> > accepts earlier in the list. However, it was already possible for an
> > asynchronous accept to cause out-of-order receives, so presumably this is
> > considered OK.
>
> I think we can simplify things if these chained packets don't follow
> the standard forwarding path, this would require to revisit many
> subsystems to handle these new chained packets - potentially a lot of
> work and likely breaking many things - and I would expect we (and
> other subsystems too) will not get very much benefits from these
> chained packets.
I guess it depends on what type of 'bundles' to expect on the wire.
For an average netfilter ruleset it will require more unbundling
because we might have
ipv4/udp -> ipv4/tcp -> ipv6/udp -> ipv4/tcp -> ipv4/udp
>From Eds patch set, this would be rebundled to
ipv4/udp -> ipv4/tcp -> ipv4/tcp -> ipv4/udp
and
ipv6/udp
nf ipv4 rule eval has to untangle again, to
ipv4/udp -> ipv4/udp
ipv4/tcp -> ipv4/tcp
HOWEVER, there are hooks that are L4 agnostic, such as defragmentation,
so we might still be able to take advantage.
Defrag is extra silly at the moment, we eat the indirect call cost
only to return NF_ACCEPT without doing anything in 99.99% of all cases.
So I still think there is value in exploring to pass the bundle
into the nf core, via new nf_hook_slow_list() that can be used
from forward path (ingress, prerouting, input, forward, postrouting).
We can make this step-by-step, first splitting everything in
nf_hook_slow_list() and just calling hooksfns for each skb in list.
> In general I like this infrastructure, but I think we can get
> something simpler if we combine it with the flowtable idea, so chained
> packets follow the non-standard flowtable forwarding path as described
> in [1].
Yes, in case all packets would be in the flow table (offload software
fallback) we might be able to keep list as-is in case everything has
same nexthop.
However, I don't see any need to make changes to this series for this
now, it can be added on top.
Did i miss anything?
I do agree that from netfilter p.o.v. ingress hook looks like a good
initial candidate for a listification.
^ permalink raw reply
* Re: [PATCHv2 net-next 1/2] route: add support for directed broadcast forwarding
From: Xin Long @ 2018-07-03 11:38 UTC (permalink / raw)
To: David Ahern; +Cc: network dev, davem, Davide Caratti, Ido Schimmel
In-Reply-To: <e4ce23df-a7ce-34d6-d457-aeb0d63b554c@gmail.com>
On Mon, Jul 2, 2018 at 11:05 PM, David Ahern <dsahern@gmail.com> wrote:
> On 7/2/18 12:30 AM, Xin Long wrote:
>> @@ -2143,6 +2149,10 @@ static int devinet_conf_proc(struct ctl_table *ctl, int write,
>> if ((new_value == 0) && (old_value != 0))
>> rt_cache_flush(net);
>>
>> + if (i == IPV4_DEVCONF_BC_FORWARDING - 1 ||
>> + new_value != old_value)
>> + rt_cache_flush(net);
>> +
>
> Shouldn't that be '&&' instead of '||' otherwise you are bumping the fib
> gen id any time the old and new values do not equal regardless of which
> setting is changed.
ah right, It's a copy-paste mistake, sorry
^ permalink raw reply
* Re: [PATCHv2 net-next 2/2] selftests: add a selftest for directed broadcast forwarding
From: Xin Long @ 2018-07-03 11:36 UTC (permalink / raw)
To: David Ahern; +Cc: network dev, davem, Davide Caratti, Ido Schimmel
In-Reply-To: <fc462436-af77-2c95-ec79-6cc418ef126e@gmail.com>
On Mon, Jul 2, 2018 at 11:12 PM, David Ahern <dsahern@gmail.com> wrote:
> On 7/2/18 12:30 AM, Xin Long wrote:
>> +ping_ipv4()
>> +{
>> + sysctl_set net.ipv4.icmp_echo_ignore_broadcasts 0
>> + bc_forwarding_disable
>> + ping_test $h1 198.51.100.255
>> +
>> + iptables -A INPUT -i vrf-r1 -p icmp -j DROP
>> + bc_forwarding_restore
>> + bc_forwarding_enable
>> + ping_test $h1 198.51.100.255
>> +
>> + bc_forwarding_restore
>> + iptables -D INPUT -i vrf-r1 -p icmp -j DROP
>> + sysctl_restore net.ipv4.icmp_echo_ignore_broadcasts
>> +}
>
> Both tests fail for me:
> TEST: ping [FAIL]
> TEST: ping [FAIL]
I think 'ip vrf exec ...' is not working in your env, while
the testing is using "ip vrf exec vrf-h1 ping ..."
You can test it by:
# ip link add dev vrf-test type vrf table 1111
# ip vrf exec vrf-test ls
>
> Why the need for the iptables rule?
This iptables rule is to block the echo_request packet going to
route's local_in.
When bc_forwarding is NOT doing forwarding well but the packet
goes to the route's local_in, it will fail.
Without this rule, the 2nd ping will always succeed, we can't tell the
echo_reply is from route or h2.
Or you have a better way to test this?
>
> And, PAUSE_ON_FAIL is not working to take a look at why tests are
> failing. e.g.,
>
> PAUSE_ON_FAIL=yes ./router_broadcast.sh
>
> just continues on. Might be something with the infrastructure scripts.
Yes, in ./router_broadcast.sh, it loads lib.sh where it loads forwarding.config
where it has "PAUSE_ON_FAIL=no", which would override your
"PAUSE_ON_FAIL=yes".
^ 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