* Re: Linux ECN Handling
From: Steve Ibanez @ 2017-10-23 22:15 UTC (permalink / raw)
To: netdev; +Cc: Florian Westphal, Mohammad Alizadeh
In-Reply-To: <20171019124312.GE16796@breakpoint.cc>
Hi All,
I upgraded the kernel on all of our machines to Linux
4.13.8-041308-lowlatency. However, I'm still observing the same
behavior where the source enters a timeout when the CWND=1MSS and it
receives ECN marks.
Here are the measured flow rates:
<https://drive.google.com/file/d/0B-bt9QS-C3ONT0VXMUt6WHhKREE/view?usp=sharing>
Here are snapshots of the packet traces at the sources when they both
enter a timeout at t=1.6sec:
10.0.0.1 timeout event:
<https://drive.google.com/file/d/0B-bt9QS-C3ONcl9WRnRPazg2ems/view?usp=sharing>
10.0.0.3 timeout event:
<https://drive.google.com/file/d/0B-bt9QS-C3ONeDlxRjNXa0VzWm8/view?usp=sharing>
Both still essentially follow the same sequence of events that I
mentioned earlier:
(1) receives an ACK for byte XYZ with the ECN flag set
(2) stops sending for RTO_min=300ms
(3) sends a retransmission for byte XYZ
The cwnd samples reported by tcp_probe still indicate that the sources
are reacting to the ECN marks more than once per window. Here are the
cwnd samples at the same timeout event mentioned above:
<https://drive.google.com/file/d/0B-bt9QS-C3ONdEZQdktpaW5JUm8/view?usp=sharing>
Let me know if there is anything else you think I should try.
Thanks,
-Steve
On Thu, Oct 19, 2017 at 5:43 AM, Florian Westphal <fw@strlen.de> wrote:
>
> [ full-quoting due to Cc fixups, adding netdev ]
>
> Steve Ibanez <sibanez@stanford.edu> wrote:
> > Hi Florian, Neal, and Daniel,
> >
> > I hope this email finds you well. My name is Stephen Ibanez and I'm a PhD
> > Student at Stanford currently working on a project with Mohammad Alizadeh,
> > Nick McKeown, and Lavanya Jose. We have been doing some experiments using
> > the linux DCTCP implementation and are trying to understand some strange
> > behavior that we are encountering. I'm contacting you three because I have
> > seen your names on some of the source files and recent commits in the linux
> > source tree. Hopefully you can help us out or put us in contact with the
> > right people?
> >
> > Here are some details about our servers:
> >
> > - Distribution: Ubuntu 14.04 LTS
> > - Kernel release: 4.4.0-75-generic
>
> Can you re-test with a more recent kernel such as 4.13.8?
>
> > *The experiment:*
> >
> > We use iperf3 to generate two DCTCP flows from different servers to a
> > common server, as shown in the diagram below. We measure the sending rate
> > of each flow, record the tcp_probe output, as well as run tcpdump on the
> > source host interfaces.
> >
> > [image: Inline image 6]
> >
> > *The problem:*
> >
> > Our rate measurements look like the one shown below; the flows often enter
> > timeouts. In this case, both flows hit a timeout at t=0.3.
> > [image: Inline image 2]
> >
> > When looking at the sequence of packets seen at the source host interfaces
> > around this timeout event this is what we see:
> >
> > *10.0.0.1 timeout event:*
> > [image: Inline image 3]
> >
> > *10.0.0.3 timeout event:*
> > [image: Inline image 4]
> >
> > In both cases, the source:
> > (1) receives an ACK for byte XYZ with the ECN flag set
> > (2) stops sending anything for RTO_min=300ms
> > (3) sends a retransmission for byte XYZ
> >
> > I have verified that this behavior is consistent across multiple experiment
> > runs. Here are the CWND samples for the 10.0.0.1 flow provided by tcp_probe
> > at the time of the timeout event:
> >
> > [image: Inline image 5]
> >
> > From what I can tell, tcp_probe logs a sample whenever a packet is
> > received. If this is true, then that means when the source receives the
> > final ECN marked ACK just before the timeout the CWND=1 MSS.
> >
> > *The conclusion:*
> >
> > We believe that there may be an issue with how the linux kernel is handling
> > the ECN echoes. For DCTCP, if the CWND is 1 MSS and the end host is still
> > receiving ECN marks then the CWND should remain at 1 MSS and should *not*
> > enter a timeout. This is because the switch can perform ECN marking very
> > aggressively causing the source end host to receive many redundant ECN
> > echoes over a short period of time.
> >
> > Another potential issue is that from the CWND plot above it looks like the
> > end host may be reacting to congestion signals more than once per window,
> > which should not happen (section 5 of RF3168
> > <https://tools.ietf.org/html/rfc3168>). tcp_probe reports SRTT measurements
> > of about 400-500 us and in the plot above the CWND is reduced 6 times
> > within this amount of time.
> >
> > We have not yet tracked down the code path in the kernel code that is
> > causing the behavior described above. Perhaps this is something that you
> > can help us with? We would love to hear your thoughts on this matter and
> > are happy to try other experiments that you suggest.
> >
> > Here is a link
> > <https://drive.google.com/file/d/0Bw-GEX7h5ufiYmpCV2VpOGEtQWs/view?usp=sharing>
> > to
> > download the packet traces if you would like to take a look.
> > han-1_host.pcap is the trace from 10.0.0.1 and han-3_host.pcap is the trace
> > from 10.0.0.3.
> >
> > Looking forward to hearing from you!
> >
> > Best,
> > -Steve
^ permalink raw reply
* [PATCH net-next 0/7] ipv6: addrconf: hash improvements and cleanups
From: Eric Dumazet @ 2017-10-23 23:17 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet
Remove unecessary BH blocking, and bring IPv6 addrconf to modern world,
with per netns hash perturbation and decent hash size.
Eric Dumazet (7):
ipv6: addrconf: move ipv6_chk_same_addr() to avoid forward declaration
ipv6: addrconf: factorize inet6_addr_hash() call
ipv6: addrconf: add per netns perturbation in inet6_addr_hash()
ipv6: addrconf: do not block BH in ipv6_chk_addr_and_flags()
ipv6: addrconf: do not block BH in ipv6_get_ifaddr()
ipv6: addrconf: do not block BH in /proc/net/if_inet6 handling
ipv6: addrconf: do not block BH in ipv6_chk_home_addr()
include/net/addrconf.h | 2 +-
net/ipv6/addrconf.c | 94 ++++++++++++++++++++++++--------------------------
2 files changed, 46 insertions(+), 50 deletions(-)
--
2.15.0.rc0.271.g36b669edcc-goog
^ permalink raw reply
* [PATCH net-next 1/7] ipv6: addrconf: move ipv6_chk_same_addr() to avoid forward declaration
From: Eric Dumazet @ 2017-10-23 23:17 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet
In-Reply-To: <20171023231751.29530-1-edumazet@google.com>
ipv6_chk_same_addr() is only used by ipv6_add_addr_hash(),
so moving it avoids a forward declaration.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv6/addrconf.c | 35 +++++++++++++++++------------------
1 file changed, 17 insertions(+), 18 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 93f9c0a619119f0520363206fbdc1aae49fbb1e6..9228030e34970c1d6380e95e8c193496448ea8ac 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -192,8 +192,6 @@ static void ipv6_ifa_notify(int event, struct inet6_ifaddr *ifa);
static void inet6_prefix_notify(int event, struct inet6_dev *idev,
struct prefix_info *pinfo);
-static bool ipv6_chk_same_addr(struct net *net, const struct in6_addr *addr,
- struct net_device *dev);
static struct ipv6_devconf ipv6_devconf __read_mostly = {
.forwarding = 0,
@@ -957,6 +955,23 @@ static u32 inet6_addr_hash(const struct in6_addr *addr)
return hash_32(ipv6_addr_hash(addr), IN6_ADDR_HSIZE_SHIFT);
}
+static bool ipv6_chk_same_addr(struct net *net, const struct in6_addr *addr,
+ struct net_device *dev)
+{
+ unsigned int hash = inet6_addr_hash(addr);
+ struct inet6_ifaddr *ifp;
+
+ hlist_for_each_entry(ifp, &inet6_addr_lst[hash], addr_lst) {
+ if (!net_eq(dev_net(ifp->idev->dev), net))
+ continue;
+ if (ipv6_addr_equal(&ifp->addr, addr)) {
+ if (!dev || ifp->idev->dev == dev)
+ return true;
+ }
+ }
+ return false;
+}
+
static int ipv6_add_addr_hash(struct net_device *dev, struct inet6_ifaddr *ifa)
{
unsigned int hash;
@@ -1856,22 +1871,6 @@ int ipv6_chk_addr_and_flags(struct net *net, const struct in6_addr *addr,
}
EXPORT_SYMBOL(ipv6_chk_addr_and_flags);
-static bool ipv6_chk_same_addr(struct net *net, const struct in6_addr *addr,
- struct net_device *dev)
-{
- unsigned int hash = inet6_addr_hash(addr);
- struct inet6_ifaddr *ifp;
-
- hlist_for_each_entry(ifp, &inet6_addr_lst[hash], addr_lst) {
- if (!net_eq(dev_net(ifp->idev->dev), net))
- continue;
- if (ipv6_addr_equal(&ifp->addr, addr)) {
- if (!dev || ifp->idev->dev == dev)
- return true;
- }
- }
- return false;
-}
/* Compares an address/prefix_len with addresses on device @dev.
* If one is found it returns true.
--
2.15.0.rc0.271.g36b669edcc-goog
^ permalink raw reply related
* [PATCH net-next 2/7] ipv6: addrconf: factorize inet6_addr_hash() call
From: Eric Dumazet @ 2017-10-23 23:17 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet
In-Reply-To: <20171023231751.29530-1-edumazet@google.com>
ipv6_add_addr_hash() can compute the hash value outside of
locked section and pass it to ipv6_chk_same_addr().
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv6/addrconf.c | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 9228030e34970c1d6380e95e8c193496448ea8ac..c1a5028f394c75d347fe703573548fd617b5c142 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -956,9 +956,8 @@ static u32 inet6_addr_hash(const struct in6_addr *addr)
}
static bool ipv6_chk_same_addr(struct net *net, const struct in6_addr *addr,
- struct net_device *dev)
+ struct net_device *dev, unsigned int hash)
{
- unsigned int hash = inet6_addr_hash(addr);
struct inet6_ifaddr *ifp;
hlist_for_each_entry(ifp, &inet6_addr_lst[hash], addr_lst) {
@@ -974,23 +973,19 @@ static bool ipv6_chk_same_addr(struct net *net, const struct in6_addr *addr,
static int ipv6_add_addr_hash(struct net_device *dev, struct inet6_ifaddr *ifa)
{
- unsigned int hash;
+ unsigned int hash = inet6_addr_hash(&ifa->addr);
int err = 0;
spin_lock(&addrconf_hash_lock);
/* Ignore adding duplicate addresses on an interface */
- if (ipv6_chk_same_addr(dev_net(dev), &ifa->addr, dev)) {
+ if (ipv6_chk_same_addr(dev_net(dev), &ifa->addr, dev, hash)) {
ADBG("ipv6_add_addr: already assigned\n");
err = -EEXIST;
- goto out;
+ } else {
+ hlist_add_head_rcu(&ifa->addr_lst, &inet6_addr_lst[hash]);
}
- /* Add to big hash table */
- hash = inet6_addr_hash(&ifa->addr);
- hlist_add_head_rcu(&ifa->addr_lst, &inet6_addr_lst[hash]);
-
-out:
spin_unlock(&addrconf_hash_lock);
return err;
--
2.15.0.rc0.271.g36b669edcc-goog
^ permalink raw reply related
* [PATCH net-next 3/7] ipv6: addrconf: add per netns perturbation in inet6_addr_hash()
From: Eric Dumazet @ 2017-10-23 23:17 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet
In-Reply-To: <20171023231751.29530-1-edumazet@google.com>
Bring IPv6 in par with IPv4 :
- Use net_hash_mix() to spread addresses a bit more.
- Use 256 slots hash table instead of 16
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/net/addrconf.h | 2 +-
net/ipv6/addrconf.c | 16 +++++++++-------
2 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index b8b16437c6d5bcb9c8b5c42b4f9c0161b477d605..15b5ffd7253d6088f2444ada5de99e4977e143ff 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -58,7 +58,7 @@ struct in6_validator_info {
struct netlink_ext_ack *extack;
};
-#define IN6_ADDR_HSIZE_SHIFT 4
+#define IN6_ADDR_HSIZE_SHIFT 8
#define IN6_ADDR_HSIZE (1 << IN6_ADDR_HSIZE_SHIFT)
int addrconf_init(void);
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index c1a5028f394c75d347fe703573548fd617b5c142..d70d98122053652b4f4420786de8788c6c9385ff 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -950,9 +950,11 @@ ipv6_link_dev_addr(struct inet6_dev *idev, struct inet6_ifaddr *ifp)
list_add_tail_rcu(&ifp->if_list, p);
}
-static u32 inet6_addr_hash(const struct in6_addr *addr)
+static u32 inet6_addr_hash(const struct net *net, const struct in6_addr *addr)
{
- return hash_32(ipv6_addr_hash(addr), IN6_ADDR_HSIZE_SHIFT);
+ u32 val = ipv6_addr_hash(addr) ^ net_hash_mix(net);
+
+ return hash_32(val, IN6_ADDR_HSIZE_SHIFT);
}
static bool ipv6_chk_same_addr(struct net *net, const struct in6_addr *addr,
@@ -973,7 +975,7 @@ static bool ipv6_chk_same_addr(struct net *net, const struct in6_addr *addr,
static int ipv6_add_addr_hash(struct net_device *dev, struct inet6_ifaddr *ifa)
{
- unsigned int hash = inet6_addr_hash(&ifa->addr);
+ unsigned int hash = inet6_addr_hash(dev_net(dev), &ifa->addr);
int err = 0;
spin_lock(&addrconf_hash_lock);
@@ -1838,8 +1840,8 @@ int ipv6_chk_addr_and_flags(struct net *net, const struct in6_addr *addr,
const struct net_device *dev, int strict,
u32 banned_flags)
{
+ unsigned int hash = inet6_addr_hash(net, addr);
struct inet6_ifaddr *ifp;
- unsigned int hash = inet6_addr_hash(addr);
u32 ifp_flags;
rcu_read_lock_bh();
@@ -1917,8 +1919,8 @@ EXPORT_SYMBOL(ipv6_chk_prefix);
struct inet6_ifaddr *ipv6_get_ifaddr(struct net *net, const struct in6_addr *addr,
struct net_device *dev, int strict)
{
+ unsigned int hash = inet6_addr_hash(net, addr);
struct inet6_ifaddr *ifp, *result = NULL;
- unsigned int hash = inet6_addr_hash(addr);
rcu_read_lock_bh();
hlist_for_each_entry_rcu_bh(ifp, &inet6_addr_lst[hash], addr_lst) {
@@ -4242,9 +4244,9 @@ void if6_proc_exit(void)
/* Check if address is a home address configured on any interface. */
int ipv6_chk_home_addr(struct net *net, const struct in6_addr *addr)
{
- int ret = 0;
+ unsigned int hash = inet6_addr_hash(net, addr);
struct inet6_ifaddr *ifp = NULL;
- unsigned int hash = inet6_addr_hash(addr);
+ int ret = 0;
rcu_read_lock_bh();
hlist_for_each_entry_rcu_bh(ifp, &inet6_addr_lst[hash], addr_lst) {
--
2.15.0.rc0.271.g36b669edcc-goog
^ permalink raw reply related
* [PATCH net-next 4/7] ipv6: addrconf: do not block BH in ipv6_chk_addr_and_flags()
From: Eric Dumazet @ 2017-10-23 23:17 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet
In-Reply-To: <20171023231751.29530-1-edumazet@google.com>
rcu_read_lock() is enough here, as inet6_ifa_finish_destroy()
uses kfree_rcu()
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv6/addrconf.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index d70d98122053652b4f4420786de8788c6c9385ff..a6cf37b7e34c461e204153078be92c3e297b3ec2 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -1844,7 +1844,7 @@ int ipv6_chk_addr_and_flags(struct net *net, const struct in6_addr *addr,
struct inet6_ifaddr *ifp;
u32 ifp_flags;
- rcu_read_lock_bh();
+ rcu_read_lock();
hlist_for_each_entry_rcu(ifp, &inet6_addr_lst[hash], addr_lst) {
if (!net_eq(dev_net(ifp->idev->dev), net))
continue;
@@ -1858,12 +1858,12 @@ int ipv6_chk_addr_and_flags(struct net *net, const struct in6_addr *addr,
!(ifp_flags&banned_flags) &&
(!dev || ifp->idev->dev == dev ||
!(ifp->scope&(IFA_LINK|IFA_HOST) || strict))) {
- rcu_read_unlock_bh();
+ rcu_read_unlock();
return 1;
}
}
- rcu_read_unlock_bh();
+ rcu_read_unlock();
return 0;
}
EXPORT_SYMBOL(ipv6_chk_addr_and_flags);
--
2.15.0.rc0.271.g36b669edcc-goog
^ permalink raw reply related
* [PATCH net-next 5/7] ipv6: addrconf: do not block BH in ipv6_get_ifaddr()
From: Eric Dumazet @ 2017-10-23 23:17 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet
In-Reply-To: <20171023231751.29530-1-edumazet@google.com>
rcu_read_lock() is enough here, no need to block BH.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv6/addrconf.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index a6cf37b7e34c461e204153078be92c3e297b3ec2..6c1e7ffb62ff2333d63b3c7639d99649d43b32fc 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -1922,8 +1922,8 @@ struct inet6_ifaddr *ipv6_get_ifaddr(struct net *net, const struct in6_addr *add
unsigned int hash = inet6_addr_hash(net, addr);
struct inet6_ifaddr *ifp, *result = NULL;
- rcu_read_lock_bh();
- hlist_for_each_entry_rcu_bh(ifp, &inet6_addr_lst[hash], addr_lst) {
+ rcu_read_lock();
+ hlist_for_each_entry_rcu(ifp, &inet6_addr_lst[hash], addr_lst) {
if (!net_eq(dev_net(ifp->idev->dev), net))
continue;
if (ipv6_addr_equal(&ifp->addr, addr)) {
@@ -1935,7 +1935,7 @@ struct inet6_ifaddr *ipv6_get_ifaddr(struct net *net, const struct in6_addr *add
}
}
}
- rcu_read_unlock_bh();
+ rcu_read_unlock();
return result;
}
--
2.15.0.rc0.271.g36b669edcc-goog
^ permalink raw reply related
* [PATCH net-next 6/7] ipv6: addrconf: do not block BH in /proc/net/if_inet6 handling
From: Eric Dumazet @ 2017-10-23 23:17 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet
In-Reply-To: <20171023231751.29530-1-edumazet@google.com>
Table is really RCU protected, no need to block BH
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv6/addrconf.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 6c1e7ffb62ff2333d63b3c7639d99649d43b32fc..9232e9537082ea44f5e0af4e4b0d052d780395fd 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -4097,9 +4097,9 @@ struct if6_iter_state {
static struct inet6_ifaddr *if6_get_first(struct seq_file *seq, loff_t pos)
{
- struct inet6_ifaddr *ifa = NULL;
struct if6_iter_state *state = seq->private;
struct net *net = seq_file_net(seq);
+ struct inet6_ifaddr *ifa = NULL;
int p = 0;
/* initial bucket if pos is 0 */
@@ -4109,7 +4109,7 @@ static struct inet6_ifaddr *if6_get_first(struct seq_file *seq, loff_t pos)
}
for (; state->bucket < IN6_ADDR_HSIZE; ++state->bucket) {
- hlist_for_each_entry_rcu_bh(ifa, &inet6_addr_lst[state->bucket],
+ hlist_for_each_entry_rcu(ifa, &inet6_addr_lst[state->bucket],
addr_lst) {
if (!net_eq(dev_net(ifa->idev->dev), net))
continue;
@@ -4135,7 +4135,7 @@ static struct inet6_ifaddr *if6_get_next(struct seq_file *seq,
struct if6_iter_state *state = seq->private;
struct net *net = seq_file_net(seq);
- hlist_for_each_entry_continue_rcu_bh(ifa, addr_lst) {
+ hlist_for_each_entry_continue_rcu(ifa, addr_lst) {
if (!net_eq(dev_net(ifa->idev->dev), net))
continue;
state->offset++;
@@ -4144,7 +4144,7 @@ static struct inet6_ifaddr *if6_get_next(struct seq_file *seq,
while (++state->bucket < IN6_ADDR_HSIZE) {
state->offset = 0;
- hlist_for_each_entry_rcu_bh(ifa,
+ hlist_for_each_entry_rcu(ifa,
&inet6_addr_lst[state->bucket], addr_lst) {
if (!net_eq(dev_net(ifa->idev->dev), net))
continue;
@@ -4157,9 +4157,9 @@ static struct inet6_ifaddr *if6_get_next(struct seq_file *seq,
}
static void *if6_seq_start(struct seq_file *seq, loff_t *pos)
- __acquires(rcu_bh)
+ __acquires(rcu)
{
- rcu_read_lock_bh();
+ rcu_read_lock();
return if6_get_first(seq, *pos);
}
@@ -4173,9 +4173,9 @@ static void *if6_seq_next(struct seq_file *seq, void *v, loff_t *pos)
}
static void if6_seq_stop(struct seq_file *seq, void *v)
- __releases(rcu_bh)
+ __releases(rcu)
{
- rcu_read_unlock_bh();
+ rcu_read_unlock();
}
static int if6_seq_show(struct seq_file *seq, void *v)
--
2.15.0.rc0.271.g36b669edcc-goog
^ permalink raw reply related
* [PATCH net-next 7/7] ipv6: addrconf: do not block BH in ipv6_chk_home_addr()
From: Eric Dumazet @ 2017-10-23 23:17 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet
In-Reply-To: <20171023231751.29530-1-edumazet@google.com>
rcu_read_lock() is enough here, no need to block BH.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv6/addrconf.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 9232e9537082ea44f5e0af4e4b0d052d780395fd..5a8a10229a07fe3d3f96b237e744332c192a509f 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -4248,8 +4248,8 @@ int ipv6_chk_home_addr(struct net *net, const struct in6_addr *addr)
struct inet6_ifaddr *ifp = NULL;
int ret = 0;
- rcu_read_lock_bh();
- hlist_for_each_entry_rcu_bh(ifp, &inet6_addr_lst[hash], addr_lst) {
+ rcu_read_lock();
+ hlist_for_each_entry_rcu(ifp, &inet6_addr_lst[hash], addr_lst) {
if (!net_eq(dev_net(ifp->idev->dev), net))
continue;
if (ipv6_addr_equal(&ifp->addr, addr) &&
@@ -4258,7 +4258,7 @@ int ipv6_chk_home_addr(struct net *net, const struct in6_addr *addr)
break;
}
}
- rcu_read_unlock_bh();
+ rcu_read_unlock();
return ret;
}
#endif
--
2.15.0.rc0.271.g36b669edcc-goog
^ permalink raw reply related
* Re: [Patch net 00/15] net_sched: remove RCU callbacks from TC
From: Cong Wang @ 2017-10-23 23:23 UTC (permalink / raw)
To: Eric Dumazet
Cc: Linux Kernel Network Developers, Paul E. McKenney,
Jamal Hadi Salim, John Fastabend, Chris Mi
In-Reply-To: <1508800599.30291.106.camel@edumazet-glaptop3.roam.corp.google.com>
On Mon, Oct 23, 2017 at 4:16 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Mon, 2017-10-23 at 15:02 -0700, Cong Wang wrote:
>
>> b) As suggested by Paul, we could defer the work to a workqueue and
>> gain the permission of holding RTNL again without any performance
>> impact, however, this seems impossible too, because as lockdep
>> complains we have a deadlock when flushing workqueue while hodling
>> RTNL lock, see the rcu_barrier() in tcf_block_put().
>>
>> Therefore, the simplest solution we have is probably just getting
>> rid of these RCU callbacks, because they are not necessary at all,
>> callers of these call_rcu() are all on slow paths and have RTNL
>> lock, so blocking is allowed in their contexts.
>
> I am against these pessimistic changes, sorry for not following past
> discussions last week.
So even Cc'ing you doesn't work. :-D
>
> I am asking a talk during upcoming netdev/netconf about this, if we need
> to take a decision.
I won't be able to make it.
>
> RTNL is already a big problem for many of us, adding synchronize_rcu()
> calls while holding RTNL is a no - go, unless we have clear evidence it
> can not be avoided.
You omitted too much, including the evidence I provide. In short it is very
hard to do, otherwise I should have already done it. I am very open to
any simple solution to avoid it, but so far none...
Saying no but without giving a possible solution does not help anything
here.
^ permalink raw reply
* Re: [PATCH net-next v7 4/5] selinux: bpf: Add selinux check for eBPF syscall operations
From: Paul Moore @ 2017-10-23 23:27 UTC (permalink / raw)
To: Chenbo Feng
Cc: netdev, SELinux, linux-security-module, Jeffrey Vander Stoep,
Alexei Starovoitov, lorenzo, Daniel Borkmann, Stephen Smalley,
James Morris, Chenbo Feng
In-Reply-To: <20171018200026.146093-5-chenbofeng.kernel@gmail.com>
On Wed, Oct 18, 2017 at 4:00 PM, Chenbo Feng
<chenbofeng.kernel@gmail.com> wrote:
> From: Chenbo Feng <fengc@google.com>
>
> Implement the actual checks introduced to eBPF related syscalls. This
> implementation use the security field inside bpf object to store a sid that
> identify the bpf object. And when processes try to access the object,
> selinux will check if processes have the right privileges. The creation
> of eBPF object are also checked at the general bpf check hook and new
> cmd introduced to eBPF domain can also be checked there.
>
> Signed-off-by: Chenbo Feng <fengc@google.com>
> Acked-by: Alexei Starovoitov <ast@kernel.org>
> ---
> security/selinux/hooks.c | 111 ++++++++++++++++++++++++++++++++++++
> security/selinux/include/classmap.h | 2 +
> security/selinux/include/objsec.h | 4 ++
> 3 files changed, 117 insertions(+)
Not sure if DaveM has merged this into net-next yet, but it looks
reasonable to me.
Acked-by: Paul Moore <paul@paul-moore.com>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index f5d304736852..12cf7de8cbed 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -85,6 +85,7 @@
> #include <linux/export.h>
> #include <linux/msg.h>
> #include <linux/shm.h>
> +#include <linux/bpf.h>
>
> #include "avc.h"
> #include "objsec.h"
> @@ -6252,6 +6253,106 @@ static void selinux_ib_free_security(void *ib_sec)
> }
> #endif
>
> +#ifdef CONFIG_BPF_SYSCALL
> +static int selinux_bpf(int cmd, union bpf_attr *attr,
> + unsigned int size)
> +{
> + u32 sid = current_sid();
> + int ret;
> +
> + switch (cmd) {
> + case BPF_MAP_CREATE:
> + ret = avc_has_perm(sid, sid, SECCLASS_BPF, BPF__MAP_CREATE,
> + NULL);
> + break;
> + case BPF_PROG_LOAD:
> + ret = avc_has_perm(sid, sid, SECCLASS_BPF, BPF__PROG_LOAD,
> + NULL);
> + break;
> + default:
> + ret = 0;
> + break;
> + }
> +
> + return ret;
> +}
> +
> +static u32 bpf_map_fmode_to_av(fmode_t fmode)
> +{
> + u32 av = 0;
> +
> + if (fmode & FMODE_READ)
> + av |= BPF__MAP_READ;
> + if (fmode & FMODE_WRITE)
> + av |= BPF__MAP_WRITE;
> + return av;
> +}
> +
> +static int selinux_bpf_map(struct bpf_map *map, fmode_t fmode)
> +{
> + u32 sid = current_sid();
> + struct bpf_security_struct *bpfsec;
> +
> + bpfsec = map->security;
> + return avc_has_perm(sid, bpfsec->sid, SECCLASS_BPF,
> + bpf_map_fmode_to_av(fmode), NULL);
> +}
> +
> +static int selinux_bpf_prog(struct bpf_prog *prog)
> +{
> + u32 sid = current_sid();
> + struct bpf_security_struct *bpfsec;
> +
> + bpfsec = prog->aux->security;
> + return avc_has_perm(sid, bpfsec->sid, SECCLASS_BPF,
> + BPF__PROG_RUN, NULL);
> +}
> +
> +static int selinux_bpf_map_alloc(struct bpf_map *map)
> +{
> + struct bpf_security_struct *bpfsec;
> +
> + bpfsec = kzalloc(sizeof(*bpfsec), GFP_KERNEL);
> + if (!bpfsec)
> + return -ENOMEM;
> +
> + bpfsec->sid = current_sid();
> + map->security = bpfsec;
> +
> + return 0;
> +}
> +
> +static void selinux_bpf_map_free(struct bpf_map *map)
> +{
> + struct bpf_security_struct *bpfsec = map->security;
> +
> + map->security = NULL;
> + kfree(bpfsec);
> +}
> +
> +static int selinux_bpf_prog_alloc(struct bpf_prog_aux *aux)
> +{
> + struct bpf_security_struct *bpfsec;
> +
> + bpfsec = kzalloc(sizeof(*bpfsec), GFP_KERNEL);
> + if (!bpfsec)
> + return -ENOMEM;
> +
> + bpfsec->sid = current_sid();
> + aux->security = bpfsec;
> +
> + return 0;
> +}
> +
> +static void selinux_bpf_prog_free(struct bpf_prog_aux *aux)
> +{
> + struct bpf_security_struct *bpfsec = aux->security;
> +
> + aux->security = NULL;
> + kfree(bpfsec);
> +}
> +#endif
> +
> static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
> LSM_HOOK_INIT(binder_set_context_mgr, selinux_binder_set_context_mgr),
> LSM_HOOK_INIT(binder_transaction, selinux_binder_transaction),
> @@ -6471,6 +6572,16 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
> LSM_HOOK_INIT(audit_rule_match, selinux_audit_rule_match),
> LSM_HOOK_INIT(audit_rule_free, selinux_audit_rule_free),
> #endif
> +
> +#ifdef CONFIG_BPF_SYSCALL
> + LSM_HOOK_INIT(bpf, selinux_bpf),
> + LSM_HOOK_INIT(bpf_map, selinux_bpf_map),
> + LSM_HOOK_INIT(bpf_prog, selinux_bpf_prog),
> + LSM_HOOK_INIT(bpf_map_alloc_security, selinux_bpf_map_alloc),
> + LSM_HOOK_INIT(bpf_prog_alloc_security, selinux_bpf_prog_alloc),
> + LSM_HOOK_INIT(bpf_map_free_security, selinux_bpf_map_free),
> + LSM_HOOK_INIT(bpf_prog_free_security, selinux_bpf_prog_free),
> +#endif
> };
>
> static __init int selinux_init(void)
> diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
> index 35ffb29a69cb..0a7023b5f000 100644
> --- a/security/selinux/include/classmap.h
> +++ b/security/selinux/include/classmap.h
> @@ -237,6 +237,8 @@ struct security_class_mapping secclass_map[] = {
> { "access", NULL } },
> { "infiniband_endport",
> { "manage_subnet", NULL } },
> + { "bpf",
> + {"map_create", "map_read", "map_write", "prog_load", "prog_run"} },
> { NULL }
> };
>
> diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h
> index 1649cd18eb0b..3d54468ce334 100644
> --- a/security/selinux/include/objsec.h
> +++ b/security/selinux/include/objsec.h
> @@ -150,6 +150,10 @@ struct pkey_security_struct {
> u32 sid; /* SID of pkey */
> };
>
> +struct bpf_security_struct {
> + u32 sid; /*SID of bpf obj creater*/
> +};
> +
> extern unsigned int selinux_checkreqprot;
>
> #endif /* _SELINUX_OBJSEC_H_ */
> --
> 2.15.0.rc1.287.g2b38de12cc-goog
>
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [PATCH net-next v7 5/5] selinux: bpf: Add addtional check for bpf object file receive
From: Paul Moore @ 2017-10-23 23:28 UTC (permalink / raw)
To: Chenbo Feng
Cc: netdev, SELinux, linux-security-module, Jeffrey Vander Stoep,
Alexei Starovoitov, lorenzo, Daniel Borkmann, Stephen Smalley,
James Morris, Chenbo Feng
In-Reply-To: <20171018200026.146093-6-chenbofeng.kernel@gmail.com>
On Wed, Oct 18, 2017 at 4:00 PM, Chenbo Feng
<chenbofeng.kernel@gmail.com> wrote:
> From: Chenbo Feng <fengc@google.com>
>
> Introduce a bpf object related check when sending and receiving files
> through unix domain socket as well as binder. It checks if the receiving
> process have privilege to read/write the bpf map or use the bpf program.
> This check is necessary because the bpf maps and programs are using a
> anonymous inode as their shared inode so the normal way of checking the
> files and sockets when passing between processes cannot work properly on
> eBPF object. This check only works when the BPF_SYSCALL is configured.
>
> Signed-off-by: Chenbo Feng <fengc@google.com>
> Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
> ---
> include/linux/bpf.h | 3 +++
> kernel/bpf/syscall.c | 4 ++--
> security/selinux/hooks.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 54 insertions(+), 2 deletions(-)
Same thing as 4/5.
Acked-by: Paul Moore <paul@paul-moore.com>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 84c192da3e0b..1e334b248ff6 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -288,6 +288,9 @@ int bpf_prog_array_copy_to_user(struct bpf_prog_array __rcu *progs,
> #ifdef CONFIG_BPF_SYSCALL
> DECLARE_PER_CPU(int, bpf_prog_active);
>
> +extern const struct file_operations bpf_map_fops;
> +extern const struct file_operations bpf_prog_fops;
> +
> #define BPF_PROG_TYPE(_id, _name) \
> extern const struct bpf_prog_ops _name ## _prog_ops; \
> extern const struct bpf_verifier_ops _name ## _verifier_ops;
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 5cb56d06b48d..323be2473c4b 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -315,7 +315,7 @@ static ssize_t bpf_dummy_write(struct file *filp, const char __user *buf,
> return -EINVAL;
> }
>
> -static const struct file_operations bpf_map_fops = {
> +const struct file_operations bpf_map_fops = {
> #ifdef CONFIG_PROC_FS
> .show_fdinfo = bpf_map_show_fdinfo,
> #endif
> @@ -975,7 +975,7 @@ static void bpf_prog_show_fdinfo(struct seq_file *m, struct file *filp)
> }
> #endif
>
> -static const struct file_operations bpf_prog_fops = {
> +const struct file_operations bpf_prog_fops = {
> #ifdef CONFIG_PROC_FS
> .show_fdinfo = bpf_prog_show_fdinfo,
> #endif
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 12cf7de8cbed..2e3a627fc0b1 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -1815,6 +1815,10 @@ static inline int file_path_has_perm(const struct cred *cred,
> return inode_has_perm(cred, file_inode(file), av, &ad);
> }
>
> +#ifdef CONFIG_BPF_SYSCALL
> +static int bpf_fd_pass(struct file *file, u32 sid);
> +#endif
> +
> /* Check whether a task can use an open file descriptor to
> access an inode in a given way. Check access to the
> descriptor itself, and then use dentry_has_perm to
> @@ -1845,6 +1849,12 @@ static int file_has_perm(const struct cred *cred,
> goto out;
> }
>
> +#ifdef CONFIG_BPF_SYSCALL
> + rc = bpf_fd_pass(file, cred_sid(cred));
> + if (rc)
> + return rc;
> +#endif
> +
> /* av is zero if only checking access to the descriptor. */
> rc = 0;
> if (av)
> @@ -2165,6 +2175,12 @@ static int selinux_binder_transfer_file(struct task_struct *from,
> return rc;
> }
>
> +#ifdef CONFIG_BPF_SYSCALL
> + rc = bpf_fd_pass(file, sid);
> + if (rc)
> + return rc;
> +#endif
> +
> if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))
> return 0;
>
> @@ -6288,6 +6304,39 @@ static u32 bpf_map_fmode_to_av(fmode_t fmode)
> return av;
> }
>
> +/* This function will check the file pass through unix socket or binder to see
> + * if it is a bpf related object. And apply correspinding checks on the bpf
> + * object based on the type. The bpf maps and programs, not like other files and
> + * socket, are using a shared anonymous inode inside the kernel as their inode.
> + * So checking that inode cannot identify if the process have privilege to
> + * access the bpf object and that's why we have to add this additional check in
> + * selinux_file_receive and selinux_binder_transfer_files.
> + */
> +static int bpf_fd_pass(struct file *file, u32 sid)
> +{
> + struct bpf_security_struct *bpfsec;
> + struct bpf_prog *prog;
> + struct bpf_map *map;
> + int ret;
> +
> + if (file->f_op == &bpf_map_fops) {
> + map = file->private_data;
> + bpfsec = map->security;
> + ret = avc_has_perm(sid, bpfsec->sid, SECCLASS_BPF,
> + bpf_map_fmode_to_av(file->f_mode), NULL);
> + if (ret)
> + return ret;
> + } else if (file->f_op == &bpf_prog_fops) {
> + prog = file->private_data;
> + bpfsec = prog->aux->security;
> + ret = avc_has_perm(sid, bpfsec->sid, SECCLASS_BPF,
> + BPF__PROG_RUN, NULL);
> + if (ret)
> + return ret;
> + }
> + return 0;
> +}
> +
> static int selinux_bpf_map(struct bpf_map *map, fmode_t fmode)
> {
> u32 sid = current_sid();
> --
> 2.15.0.rc1.287.g2b38de12cc-goog
>
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [Patch net 00/15] net_sched: remove RCU callbacks from TC
From: Eric Dumazet @ 2017-10-23 23:31 UTC (permalink / raw)
To: Cong Wang
Cc: Linux Kernel Network Developers, Paul E. McKenney,
Jamal Hadi Salim, John Fastabend, Chris Mi
In-Reply-To: <CAM_iQpUXr-3rAkBiVFvZ5YSRG=ZrPEWzQwQYZ0Yx_YP_BQNGoA@mail.gmail.com>
On Mon, 2017-10-23 at 16:23 -0700, Cong Wang wrote:
> On Mon, Oct 23, 2017 at 4:16 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > On Mon, 2017-10-23 at 15:02 -0700, Cong Wang wrote:
> >
> >> b) As suggested by Paul, we could defer the work to a workqueue and
> >> gain the permission of holding RTNL again without any performance
> >> impact, however, this seems impossible too, because as lockdep
> >> complains we have a deadlock when flushing workqueue while hodling
> >> RTNL lock, see the rcu_barrier() in tcf_block_put().
> >>
> >> Therefore, the simplest solution we have is probably just getting
> >> rid of these RCU callbacks, because they are not necessary at all,
> >> callers of these call_rcu() are all on slow paths and have RTNL
> >> lock, so blocking is allowed in their contexts.
> >
> > I am against these pessimistic changes, sorry for not following past
> > discussions last week.
>
> So even Cc'ing you doesn't work. :-D
Nope. At the end of the day, there are only 24 hours per day.
>
> >
> > I am asking a talk during upcoming netdev/netconf about this, if we need
> > to take a decision.
>
> I won't be able to make it.
>
> >
> > RTNL is already a big problem for many of us, adding synchronize_rcu()
> > calls while holding RTNL is a no - go, unless we have clear evidence it
> > can not be avoided.
>
> You omitted too much, including the evidence I provide. In short it is very
> hard to do, otherwise I should have already done it. I am very open to
> any simple solution to avoid it, but so far none...
>
> Saying no but without giving a possible solution does not help anything
> here.
I did not pretend to give a bug fix, I simply said your patch series was
probably not the right way.
Sure, we could add back BKL and solve a lot of lockdep issues.
^ permalink raw reply
* Hello
From: Mrs. Liz @ 2017-10-24 0:06 UTC (permalink / raw)
To: Recipients
Dear Sir/Madam
Greetings to you My name is Mrs. Liz Clayton I am 73 years old, I am a dying woman who has decided to donate what I have to you/churches/ motherless babies/less privileged/widows.I was diagnosed with cancer for about 2 years ago. I have been touched by God to donate from what I have inherited from my late husband to you for good work of God. Please if you are ready to assist me to distribute my funds of Twenty Million Dollars to charity. Awaiting your reply.
Your sister in Christ,
Mrs. Liz Clayton.
^ permalink raw reply
* Re: [PATCH 0/3] pull request for net-next: batman-adv 2017-10-23
From: David Miller @ 2017-10-24 0:15 UTC (permalink / raw)
To: sw; +Cc: netdev, b.a.t.m.a.n
In-Reply-To: <20171023153704.32120-1-sw@simonwunderlich.de>
From: Simon Wunderlich <sw@simonwunderlich.de>
Date: Mon, 23 Oct 2017 17:37:01 +0200
> here is another small documentation/cleanup pull request of batman-adv to go
> into net-next.
>
> Please pull or let me know of any problem!
Pulled, thanks Simon.
^ permalink raw reply
* Re: [PATCH net-next 0/7] net: hns3: bug fixes & code improvements
From: David Miller @ 2017-10-24 0:17 UTC (permalink / raw)
To: lipeng321; +Cc: netdev, linux-kernel, linuxarm, yisen.zhuang, salil.mehta
In-Reply-To: <1508759467-18397-1-git-send-email-lipeng321@huawei.com>
From: Lipeng <lipeng321@huawei.com>
Date: Mon, 23 Oct 2017 19:51:00 +0800
> This patchset introduces various HNS3 bug fixes, optimizations and code
> improvements.
Series applied.
^ permalink raw reply
* Re: [PATCH net-next 00/12] tools: bpftool: Add JSON output to bpftool
From: David Miller @ 2017-10-24 0:25 UTC (permalink / raw)
To: jakub.kicinski; +Cc: netdev, oss-drivers, alexei.starovoitov, daniel
In-Reply-To: <20171023162416.32753-1-jakub.kicinski@netronome.com>
From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Mon, 23 Oct 2017 09:24:04 -0700
> Quentin says:
>
> This series introduces support for JSON output to all bpftool commands. It
> adds option parsing, and several options are created:
>
> * -j, --json Switch to JSON output.
> * -p, --pretty Switch to JSON and print it in a human-friendly fashion.
> * -h, --help Print generic help message.
> * -V, --version Print version number.
>
> This code uses a "json_writer", which is a copy of the one written by
> Stephen Hemminger in iproute2.
> ---
> I don't know if there is an easy way to share the code for json_write
> without copying the file, so I am very open to suggestions on this matter.
Series applied.
^ permalink raw reply
* Re: [PATCH v2 net-next 0/6] add a set of tracepoints to tcp stack
From: David Miller @ 2017-10-24 0:25 UTC (permalink / raw)
To: songliubraving; +Cc: netdev, alexei.starovoitov, liu.song.a23
In-Reply-To: <20171023162027.2880430-1-songliubraving@fb.com>
From: Song Liu <songliubraving@fb.com>
Date: Mon, 23 Oct 2017 09:20:21 -0700
> Changes from v1:
>
> Fix build error (with ipv6 as ko) by adding EXPORT_TRACEPOINT_SYMBOL_GPL
> for trace_tcp_send_reset.
Series applied.
^ permalink raw reply
* RE: [PATCH] drivers/net/usb: add device id for TP-LINK UE300 USB 3.0 Ethernet
From: Ran Wang @ 2017-10-24 0:54 UTC (permalink / raw)
To: Oliver Neukum
Cc: David S . Miller, hayeswang, linux-kernel@vger.kernel.org,
linux-usb@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <1508756170.28673.7.camel@suse.com>
Hi Oliver
> -----Original Message-----
> From: Oliver Neukum [mailto:oneukum@suse.com]
> Sent: Monday, October 23, 2017 6:56 PM
> To: Ran Wang <ran.wang_1@nxp.com>
> Cc: David S . Miller <davem@davemloft.net>; hayeswang
> <hayeswang@realtek.com>; linux-kernel@vger.kernel.org; linux-
> usb@vger.kernel.org; netdev@vger.kernel.org
> Subject: Re: [PATCH] drivers/net/usb: add device id for TP-LINK UE300 USB
> 3.0 Ethernet
>
> Am Montag, den 23.10.2017, 18:10 +0800 schrieb Ran Wang:
> > This product is named 'TP-LINK USB 3.0 Gigabit Ethernet Network
> > Adapter (Model No.is UE300)'. It uses chip RTL8153 and works with
> > driver drivers/net/usb/r8152.c
> >
>
> Hi,
>
> just for the record, have you confirm that it fails with cdc-ether?
>
Yes, when I use default cdc-ether and do data transfer test, I see system flooding message
'[ 1076.627366] cdc_ether 4-1:2.0 eth8: kevent 11 may have been dropped' and related eth#
Shows most packets received has been dropped. After load r8152.c, everything work fine.
BR
Ran
> Regards
> Oliver
^ permalink raw reply
* Re: [PATCH net-next 0/2] net: dsa: don't unmask port bitmaps
From: Florian Fainelli @ 2017-10-24 0:54 UTC (permalink / raw)
To: Vivien Didelot, Andrew Lunn; +Cc: netdev, linux-kernel, kernel, David S. Miller
In-Reply-To: <87h8upeesg.fsf@weeman.i-did-not-set--mail-host-address--so-tickle-me>
On 10/23/2017 02:26 PM, Vivien Didelot wrote:
> Hi Andrew,
>
> Andrew Lunn <andrew@lunn.ch> writes:
>
>> On Mon, Oct 23, 2017 at 02:17:29PM -0400, Vivien Didelot wrote:
>>> DSA has several bitmaps to store the type of ports: cpu_port_mask,
>>> dsa_port_mask and enabled_port_mask. But the code is inconsistently
>>> unmasking them.
>>>
>>> The legacy code tries to unmask cpu_port_mask and dsa_port_mask but
>>> skips enabled_port_mask.
>>>
>>> The new bindings unmasks cpu_port_mask and enabled_port_mask but skips
>>> dsa_port_mask.
>>>
>>> In fact there is no need to unmask them because we are in the error
>>> path, and they won't be used after. Instead of fixing the unmasking,
>>> simply remove them.
>>
>> I'm not looked at the code, travelling and don't have time.
>
> heu, ok.
>
>> What happens if the failure is -PROBE_DEFERRED, and it tried again
>> later. Will these masks be set back to 0? Or will they retain the old
>> values? I think there is supposed to be symmetry here, so that we undo
>> what we did, and so the next time we try again, we start from a good
>> state.
In case of probe deferral, you get the full probe function to exit with
an error, and that usually involves freeing the recently allocated
dsa_switch instance, and then allocating a new one when probe is
re-entered, so that should not be a problem.
>
> The type of a port is a static information parsed either from device
> tree or from platform data. Thus there is no symmetry needed here.
>
> The fact that the unmasking of these bitmaps is currently erroneous also
> shows that it is unnecessary.
As much as I would like to see this being symmetrical here, as Vivien
points out, this does not appear to be the case because of missing code,
and it does not seem to solve a particular problem in being symmetrical.
--
Florian
^ permalink raw reply
* Re: Linux ECN Handling
From: Neal Cardwell @ 2017-10-24 1:11 UTC (permalink / raw)
To: Steve Ibanez; +Cc: Netdev, Florian Westphal, Mohammad Alizadeh
In-Reply-To: <CACJspmKjAr+q9cFVssXVxWQMCUWe3TNYO77m0nQwzQK4hTCOzA@mail.gmail.com>
On Mon, Oct 23, 2017 at 6:15 PM, Steve Ibanez <sibanez@stanford.edu> wrote:
> Hi All,
>
> I upgraded the kernel on all of our machines to Linux
> 4.13.8-041308-lowlatency. However, I'm still observing the same
> behavior where the source enters a timeout when the CWND=1MSS and it
> receives ECN marks.
>
> Here are the measured flow rates:
> <https://drive.google.com/file/d/0B-bt9QS-C3ONT0VXMUt6WHhKREE/view?usp=sharing>
>
> Here are snapshots of the packet traces at the sources when they both
> enter a timeout at t=1.6sec:
>
> 10.0.0.1 timeout event:
> <https://drive.google.com/file/d/0B-bt9QS-C3ONcl9WRnRPazg2ems/view?usp=sharing>
>
> 10.0.0.3 timeout event:
> <https://drive.google.com/file/d/0B-bt9QS-C3ONeDlxRjNXa0VzWm8/view?usp=sharing>
>
> Both still essentially follow the same sequence of events that I
> mentioned earlier:
> (1) receives an ACK for byte XYZ with the ECN flag set
> (2) stops sending for RTO_min=300ms
> (3) sends a retransmission for byte XYZ
>
> The cwnd samples reported by tcp_probe still indicate that the sources
> are reacting to the ECN marks more than once per window. Here are the
> cwnd samples at the same timeout event mentioned above:
> <https://drive.google.com/file/d/0B-bt9QS-C3ONdEZQdktpaW5JUm8/view?usp=sharing>
>
> Let me know if there is anything else you think I should try.
Sounds like perhaps cwnd is being set to 0 somewhere in this DCTCP
scenario. Would you be able to add printk statements in
tcp_init_cwnd_reduction(), tcp_cwnd_reduction(), and
tcp_end_cwnd_reduction(), printing the IP:port, tp->snd_cwnd, and
tp->snd_ssthresh?
Based on the output you may be able to figure out where cwnd is being
set to zero. If not, could you please post the printk output and
tcpdump traces (.pcap, headers-only is fine) from your tests?
thanks,
neal
^ permalink raw reply
* [PATCH net] xfrm: Clear sk_dst_cache when applying per-socket policy.
From: Jonathan Basseri @ 2017-10-24 1:18 UTC (permalink / raw)
To: netdev
Cc: jkbs, davem, steffen.klassert, lorenzo, eric.dumazet,
Jonathan Basseri
In-Reply-To: <20170815222510.21711-1-misterikkit@google.com>
If a socket has a valid dst cache, then xfrm_lookup_route will get
skipped. However, the cache is not invalidated when applying policy to a
socket (i.e. IPV6_XFRM_POLICY). The result is that new policies are
sometimes ignored on those sockets. (Note: This was broken for IPv4 and
IPv6 at different times.)
This can be demonstrated like so,
1. Create UDP socket.
2. connect() the socket.
3. Apply an outbound XFRM policy to the socket.
4. send() data on the socket.
Packets will continue to be sent in the clear instead of matching an
xfrm or returning a no-match error (EAGAIN). This affects calls to
send() and not sendto().
Invalidating the sk_dst_cache is necessary to correctly apply xfrm
policies. Since we do this in xfrm_user_policy(), the sk_lock was
already acquired in either do_ip_setsockopt() or do_ipv6_setsockopt(),
and we may call __sk_dst_reset().
Performance impact should be negligible, since this code is only called
when changing xfrm policy, and only affects the socket in question.
Note: Creating normal XFRM policies should have a similar effect on
sk_dst_cache entries that match the policy, but that is not fixed in
this patch.
Fixes: 00bc0ef5880d ("ipv6: Skip XFRM lookup if dst_entry in socket cache is valid")
Tested: https://android-review.googlesource.com/517555
Tested: https://android-review.googlesource.com/418659
Signed-off-by: Jonathan Basseri <misterikkit@google.com>
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 12213477cd3a..1f5cee2269af 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -2045,33 +2045,34 @@ EXPORT_SYMBOL(km_is_alive);
int xfrm_user_policy(struct sock *sk, int optname, u8 __user *optval, int optlen)
{
int err;
u8 *data;
struct xfrm_mgr *km;
struct xfrm_policy *pol = NULL;
if (optlen <= 0 || optlen > PAGE_SIZE)
return -EMSGSIZE;
data = memdup_user(optval, optlen);
if (IS_ERR(data))
return PTR_ERR(data);
err = -EINVAL;
rcu_read_lock();
list_for_each_entry_rcu(km, &xfrm_km_list, list) {
pol = km->compile_policy(sk, optname, data,
optlen, &err);
if (err >= 0)
break;
}
rcu_read_unlock();
if (err >= 0) {
xfrm_sk_policy_insert(sk, err, pol);
xfrm_pol_put(pol);
+ __sk_dst_reset(sk);
err = 0;
}
kfree(data);
return err;
}
--
2.15.0.rc0.271.g36b669edcc-goog
^ permalink raw reply related
* Re: [PATCH net] xfrm: Clear sk_dst_cache when applying per-socket policy.
From: Jonathan Basseri 😶 @ 2017-10-24 1:30 UTC (permalink / raw)
To: netdev
Cc: Jakub Sitnicki, David Miller, Steffen Klassert, Lorenzo Colitti,
Eric Dumazet, Jonathan Basseri
On Mon, Oct 23, 2017 at 6:18 PM, Jonathan Basseri
<misterikkit@google.com> wrote:
> If a socket has a valid dst cache, then xfrm_lookup_route will get
> skipped. However, the cache is not invalidated when applying policy to a
> socket (i.e. IPV6_XFRM_POLICY). The result is that new policies are
> sometimes ignored on those sockets. (Note: This was broken for IPv4 and
> IPv6 at different times.)
>
> This can be demonstrated like so,
> 1. Create UDP socket.
> 2. connect() the socket.
> 3. Apply an outbound XFRM policy to the socket.
> 4. send() data on the socket.
>
> Packets will continue to be sent in the clear instead of matching an
> xfrm or returning a no-match error (EAGAIN). This affects calls to
> send() and not sendto().
>
> Invalidating the sk_dst_cache is necessary to correctly apply xfrm
> policies. Since we do this in xfrm_user_policy(), the sk_lock was
> already acquired in either do_ip_setsockopt() or do_ipv6_setsockopt(),
> and we may call __sk_dst_reset().
>
> Performance impact should be negligible, since this code is only called
> when changing xfrm policy, and only affects the socket in question.
>
> Note: Creating normal XFRM policies should have a similar effect on
> sk_dst_cache entries that match the policy, but that is not fixed in
> this patch.
>
> Fixes: 00bc0ef5880d ("ipv6: Skip XFRM lookup if dst_entry in socket cache is valid")
> Tested: https://android-review.googlesource.com/517555
> Tested: https://android-review.googlesource.com/418659
> Signed-off-by: Jonathan Basseri <misterikkit@google.com>
>
> diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
> index 12213477cd3a..1f5cee2269af 100644
> --- a/net/xfrm/xfrm_state.c
> +++ b/net/xfrm/xfrm_state.c
> @@ -2045,33 +2045,34 @@ EXPORT_SYMBOL(km_is_alive);
> int xfrm_user_policy(struct sock *sk, int optname, u8 __user *optval, int optlen)
> {
> int err;
> u8 *data;
> struct xfrm_mgr *km;
> struct xfrm_policy *pol = NULL;
>
> if (optlen <= 0 || optlen > PAGE_SIZE)
> return -EMSGSIZE;
>
> data = memdup_user(optval, optlen);
> if (IS_ERR(data))
> return PTR_ERR(data);
>
> err = -EINVAL;
> rcu_read_lock();
> list_for_each_entry_rcu(km, &xfrm_km_list, list) {
> pol = km->compile_policy(sk, optname, data,
> optlen, &err);
> if (err >= 0)
> break;
> }
> rcu_read_unlock();
>
> if (err >= 0) {
> xfrm_sk_policy_insert(sk, err, pol);
> xfrm_pol_put(pol);
> + __sk_dst_reset(sk);
> err = 0;
> }
>
> kfree(data);
> return err;
> }
> --
> 2.15.0.rc0.271.g36b669edcc-goog
>
I discussed the concerns with Eric and I believe this addresses them.
(http://www.spinics.net/lists/netdev/msg449652.html)
^ permalink raw reply
* Re: [PATCH 0/2] 9p: Fixes for hard-to-hit bugs
From: Tuomas Tynkkynen @ 2017-10-24 1:41 UTC (permalink / raw)
To: Al Viro
Cc: v9fs-developer, Eric Van Hensbergen, Ron Minnich,
Latchesar Ionkov, David S. Miller, linux-kernel, netdev,
linux-fsdevel
In-Reply-To: <20171020201125.GU21978@ZenIV.linux.org.uk>
Hi Al,
On Fri, 2017-10-20 at 21:11 +0100, Al Viro wrote:
> On Tue, Sep 26, 2017 at 04:10:14PM +0300, Tuomas Tynkkynen wrote:
> > Hi Al,
> >
> > On Wed, 2017-09-06 at 17:59 +0300, Tuomas Tynkkynen wrote:
> > > These two patches fix two hard-to-hit (but really annoying) bugs
> > > in
> > > 9p.
> > > The first one was posted earlier in February (with one R-b), the
> > > second
> > > is a new one.
> > >
...
> > >
> >
> > Could you apply these? Thanks!
>
> I can pick those, or, if you (or somebody else) are willing to
> actively
> maintain a 9p tree, you could start sending straight to Linus - up to
> you.
You can pick these up, I don't have plans for more patches right now
(unless we hit some new bugs that affect our workloads).
(FWIW there do lurk dragons there, at least 1) setxattr() with zero-
length value turns into a removexattr(), and 2) creat("foo", 0444)
fails due to the R/O perms when opening the writeback_fid. Fixes to
either didn't seem too easy...)
Thanks!
- Tuomas
^ permalink raw reply
* [PATCH] rtlwifi: rtl8821ae: Fix typo in variable name
From: Nik Nyby @ 2017-10-24 1:49 UTC (permalink / raw)
To: Larry.Finger, kvalo, linux-wireless, netdev, trivial; +Cc: Nik Nyby
In _rtl8821ae_dbi_write(), wrtie_addr should be write_addr.
Signed-off-by: Nik Nyby <nikolas@gnu.org>
---
drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c
index 1d431d4bf6d2..6a9d8ee0ae9f 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c
@@ -1130,13 +1130,13 @@ static u8 _rtl8821ae_dbi_read(struct rtl_priv *rtlpriv, u16 addr)
static void _rtl8821ae_dbi_write(struct rtl_priv *rtlpriv, u16 addr, u8 data)
{
u8 tmp = 0, count = 0;
- u16 wrtie_addr, remainder = addr % 4;
+ u16 write_addr, remainder = addr % 4;
- wrtie_addr = REG_DBI_WDATA + remainder;
- rtl_write_byte(rtlpriv, wrtie_addr, data);
+ write_addr = REG_DBI_WDATA + remainder;
+ rtl_write_byte(rtlpriv, write_addr, data);
- wrtie_addr = (addr & 0xfffc) | (BIT(0) << (remainder + 12));
- rtl_write_word(rtlpriv, REG_DBI_ADDR, wrtie_addr);
+ write_addr = (addr & 0xfffc) | (BIT(0) << (remainder + 12));
+ rtl_write_word(rtlpriv, REG_DBI_ADDR, write_addr);
rtl_write_byte(rtlpriv, REG_DBI_FLAG, 0x1);
--
2.14.2
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox