* [PATCH v2 net-next 1/2] tcp: show number of network segments in some SNMP counters
From: Yafang Shao @ 2018-09-09 3:14 UTC (permalink / raw)
To: edumazet, davem; +Cc: netdev, linux-kernel, Yafang Shao
It is better to show the number of network segments in bellow SNMP
counters, because that could be more useful for the user.
For example, the user could easily figure out how mant packets are
dropped and how many packets are queued in the out-of-oder queue.
- LINUX_MIB_TCPRCVQDROP
- LINUX_MIB_TCPZEROWINDOWDROP
- LINUX_MIB_TCPBACKLOGDROP
- LINUX_MIB_TCPMINTTLDROP
- LINUX_MIB_TCPOFODROP
- LINUX_MIB_TCPOFOQUEUE
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
net/ipv4/tcp_input.c | 18 ++++++++++++------
net/ipv4/tcp_ipv4.c | 9 ++++++---
net/ipv6/tcp_ipv6.c | 6 ++++--
3 files changed, 22 insertions(+), 11 deletions(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 62508a2..c2ce334 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -4496,7 +4496,8 @@ static void tcp_data_queue_ofo(struct sock *sk, struct sk_buff *skb)
tcp_ecn_check_ce(sk, skb);
if (unlikely(tcp_try_rmem_schedule(sk, skb, skb->truesize))) {
- NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPOFODROP);
+ NET_ADD_STATS(sock_net(sk), LINUX_MIB_TCPOFODROP,
+ max_t(u16, 1, skb_shinfo(skb)->gso_segs));
tcp_drop(sk, skb);
return;
}
@@ -4505,7 +4506,8 @@ static void tcp_data_queue_ofo(struct sock *sk, struct sk_buff *skb)
tp->pred_flags = 0;
inet_csk_schedule_ack(sk);
- NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPOFOQUEUE);
+ NET_ADD_STATS(sock_net(sk), LINUX_MIB_TCPOFOQUEUE,
+ max_t(u16, 1, skb_shinfo(skb)->gso_segs));
seq = TCP_SKB_CB(skb)->seq;
end_seq = TCP_SKB_CB(skb)->end_seq;
SOCK_DEBUG(sk, "out of order segment: rcv_next %X seq %X - %X\n",
@@ -4666,7 +4668,8 @@ int tcp_send_rcvq(struct sock *sk, struct msghdr *msg, size_t size)
skb->len = size;
if (tcp_try_rmem_schedule(sk, skb, skb->truesize)) {
- NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPRCVQDROP);
+ NET_ADD_STATS(sock_net(sk), LINUX_MIB_TCPRCVQDROP,
+ max_t(u16, 1, skb_shinfo(skb)->gso_segs));
goto err_free;
}
@@ -4725,7 +4728,8 @@ static void tcp_data_queue(struct sock *sk, struct sk_buff *skb)
*/
if (TCP_SKB_CB(skb)->seq == tp->rcv_nxt) {
if (tcp_receive_window(tp) == 0) {
- NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPZEROWINDOWDROP);
+ NET_ADD_STATS(sock_net(sk), LINUX_MIB_TCPZEROWINDOWDROP,
+ max_t(u16, 1, skb_shinfo(skb)->gso_segs));
goto out_of_window;
}
@@ -4734,7 +4738,8 @@ static void tcp_data_queue(struct sock *sk, struct sk_buff *skb)
if (skb_queue_len(&sk->sk_receive_queue) == 0)
sk_forced_mem_schedule(sk, skb->truesize);
else if (tcp_try_rmem_schedule(sk, skb, skb->truesize)) {
- NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPRCVQDROP);
+ NET_ADD_STATS(sock_net(sk), LINUX_MIB_TCPRCVQDROP,
+ max_t(u16, 1, skb_shinfo(skb)->gso_segs));
goto drop;
}
@@ -4796,7 +4801,8 @@ static void tcp_data_queue(struct sock *sk, struct sk_buff *skb)
* remembering D-SACK for its head made in previous line.
*/
if (!tcp_receive_window(tp)) {
- NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPZEROWINDOWDROP);
+ NET_ADD_STATS(sock_net(sk), LINUX_MIB_TCPZEROWINDOWDROP,
+ max_t(u16, 1, skb_shinfo(skb)->gso_segs));
goto out_of_window;
}
goto queue_and_out;
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 09547ef..23d7cb5 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -475,7 +475,8 @@ void tcp_v4_err(struct sk_buff *icmp_skb, u32 info)
goto out;
if (unlikely(iph->ttl < inet_sk(sk)->min_ttl)) {
- __NET_INC_STATS(net, LINUX_MIB_TCPMINTTLDROP);
+ __NET_ADD_STATS(net, LINUX_MIB_TCPMINTTLDROP,
+ max_t(u16, 1, skb_shinfo(skb)->gso_segs));
goto out;
}
@@ -1633,7 +1634,8 @@ bool tcp_add_backlog(struct sock *sk, struct sk_buff *skb)
if (unlikely(sk_add_backlog(sk, skb, limit))) {
bh_unlock_sock(sk);
- __NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPBACKLOGDROP);
+ __NET_ADD_STATS(sock_net(sk), LINUX_MIB_TCPBACKLOGDROP,
+ max_t(u16, 1, skb_shinfo(skb)->gso_segs));
return true;
}
return false;
@@ -1790,7 +1792,8 @@ int tcp_v4_rcv(struct sk_buff *skb)
}
}
if (unlikely(iph->ttl < inet_sk(sk)->min_ttl)) {
- __NET_INC_STATS(net, LINUX_MIB_TCPMINTTLDROP);
+ __NET_ADD_STATS(net, LINUX_MIB_TCPMINTTLDROP,
+ max_t(u16, 1, skb_shinfo(skb)->gso_segs));
goto discard_and_relse;
}
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 03e6b7a..bbf7667 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -391,7 +391,8 @@ static void tcp_v6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
goto out;
if (ipv6_hdr(skb)->hop_limit < inet6_sk(sk)->min_hopcount) {
- __NET_INC_STATS(net, LINUX_MIB_TCPMINTTLDROP);
+ __NET_ADD_STATS(net, LINUX_MIB_TCPMINTTLDROP,
+ max_t(u16, 1, skb_shinfo(skb)->gso_segs));
goto out;
}
@@ -1523,7 +1524,8 @@ static int tcp_v6_rcv(struct sk_buff *skb)
}
}
if (hdr->hop_limit < inet6_sk(sk)->min_hopcount) {
- __NET_INC_STATS(net, LINUX_MIB_TCPMINTTLDROP);
+ __NET_ADD_STATS(net, LINUX_MIB_TCPMINTTLDROP,
+ max_t(u16, 1, skb_shinfo(skb)->gso_segs));
goto discard_and_relse;
}
--
1.8.3.1
^ permalink raw reply related
* [PATCH v2 net-next 2/2] tcp: fix the error count of tcpInSegs
From: Yafang Shao @ 2018-09-09 3:14 UTC (permalink / raw)
To: edumazet, davem; +Cc: netdev, linux-kernel, Yafang Shao
In-Reply-To: <1536462862-11767-1-git-send-email-laoar.shao@gmail.com>
In RFC1213, the tcpInSegs is the total number of segments received.
While currently it is the total number of SKBs received.
The number of SKBs may be not equal with the numer of segments because of
GRO.
So fix this error count.
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
include/net/tcp.h | 2 ++
net/ipv4/tcp_ipv4.c | 3 ++-
net/ipv6/tcp_ipv6.c | 3 ++-
3 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 770917d..66578f4 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -310,6 +310,8 @@ static inline bool tcp_too_many_orphans(struct sock *sk, int shift)
#define __TCP_INC_STATS(net, field) __SNMP_INC_STATS((net)->mib.tcp_statistics, field)
#define TCP_DEC_STATS(net, field) SNMP_DEC_STATS((net)->mib.tcp_statistics, field)
#define TCP_ADD_STATS(net, field, val) SNMP_ADD_STATS((net)->mib.tcp_statistics, field, val)
+#define __TCP_ADD_STATS(net, field, val) \
+ __SNMP_ADD_STATS((net)->mib.tcp_statistics, field, val)
void tcp_tasklet_init(void);
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 23d7cb5..2b98242 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1703,7 +1703,8 @@ int tcp_v4_rcv(struct sk_buff *skb)
goto discard_it;
/* Count it even if it's bad */
- __TCP_INC_STATS(net, TCP_MIB_INSEGS);
+ __TCP_ADD_STATS(net, TCP_MIB_INSEGS,
+ max_t(u16, 1, skb_shinfo(skb)->gso_segs));
if (!pskb_may_pull(skb, sizeof(struct tcphdr)))
goto discard_it;
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index bbf7667..8d4ef46 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1441,7 +1441,8 @@ static int tcp_v6_rcv(struct sk_buff *skb)
/*
* Count it even if it's bad.
*/
- __TCP_INC_STATS(net, TCP_MIB_INSEGS);
+ __TCP_ADD_STATS(net, TCP_MIB_INSEGS,
+ max_t(u16, 1, skb_shinfo(skb)->gso_segs));
if (!pskb_may_pull(skb, sizeof(struct tcphdr)))
goto discard_it;
--
1.8.3.1
^ permalink raw reply related
* Re: Why not use all the syn queues? in the function "tcp_conn_request", I have some questions.
From: Ttttabcd @ 2018-09-09 1:14 UTC (permalink / raw)
To: Neal Cardwell; +Cc: Netdev
In-Reply-To: <CADVnQymHJ5VGpawQMWtcvO6YUTC-tV9bWcb0meu=c0MKmQdyhQ@mail.gmail.com>
Sent with ProtonMail Secure Email.
‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Sunday, 9 September 2018 02:24, Neal Cardwell <ncardwell@google.com> wrote:
> By default, and essentially always in practice (AFAIK), Linux
> installations enable syncookies. With syncookies, there is essentially
> no limit on the syn queue, or number of incomplete passive connections
> (as the man page you quoted notes). So in practice the listen()
> parameter usually controls only the accept queue.
>
>
> That discussion pertains to a code path that is relevant if syncookies
> are disabled, which is very uncommon (see above).
>
Yes, when I tested, I disabled syncookies. I want to know how the kernel will handle syn attacks if syncookies are disabled.
> Keep in mind that the semantics of the listen() argument and the
> /proc/sys/net/ipv4/tcp_max_syn_backlog sysctl knob, as described in
> the man page, are part of the Linux kernel's user-visible API. So, in
> essence, they cannot be changed. Changing the semantics of system
> calls and sysctl knobs breaks applications and system configuration
> scripts. :-)
So, as you said
Is there a historical issue with two variables controlling the syn queue?
^ permalink raw reply
* Re: [PATCH net-next] veth: report NEWLINK event when moving the peer device in a new namespace
From: David Ahern @ 2018-09-09 1:16 UTC (permalink / raw)
To: Thomas Haller, Lorenzo Bianconi; +Cc: David S. Miller, Network Development
In-Reply-To: <8eb1f295bbfe0d662e0df19448e9719be24348f5.camel@redhat.com>
Hi Thomas:
On 9/7/18 12:52 PM, Thomas Haller wrote:
> Hi David,
>
>
> On Mon, 2018-09-03 at 20:54 -0600, David Ahern wrote:
>
>> From init_net:
>> $ ip monitor all-nsid
>
> I thought the concern of the patch is the overhead of sending one
> additional RTM_NEWLINK message. This workaround has likely higher
> overhead. More importantly, it's so cumbersome, that I doubt anybody
> would implementing such a solution.
Yes, the concern is additional messages. Each one adds non-negligible
overhead to build and send, something that is measurable in a system at
scale. For example, consider an interface manager creating 4k VLANs on
one or more bridges. The RTM_NEWLINK messages (there are 2) adds 33% to
the overhead and time of creating the vlans. Just in some quick tests I
see times vary from as little as 15 usec to over 1 msec - per message
with the RTNL held. The norm seems to be somewhere between 35 and 40
usecs, but those add up when it is per link and per action.
We have to more disciplined about the size and frequency of these messages.
>
> When the events of one namespace are not sufficient to get all relevant
> information (local to the namespace itself), the solution is not
> monitor all-nsid.
>
> You might save complexity and performance overhead in kernel. But what
> you save here is just moved to user-space, which faces higher
> complexity (at multiple places/projects, where developers are not
> experts in netlink) and higher overhead.
First, this is one use case that seems to care about a message coming
back on the veth pairs. The message is sent for everyone, always. Adding
a 3rd message puts additional load on all use cases.
Second, I am questioning this use case driving a kernel change. You say
a userspace app cares about a veth pair but only wants to monitor events
for one end of it. That's a userspace choice. Another argument brought
up is that the other end of the pair can change namespaces faster than
the app can track it. Sure. But the monitored end can move as well - and
faster than the app can track it. e.g.,
1. veth1 and veth2 are a pair.
2. app filters events for veth1 only
3. veth2 is moved to namespace foo
4. app is sent notification of move
5. veth2 is moved to namespace bar
6. app processes notification of the event in step 3, looks in namespace
foo and link is gone.
That seems to be the fundamentals of your request for this new message, yes?
What happens when veth1 is moved to another namespace and then another
namespace - faster than the app can not track it? You have the same
problem. Your use case may not have this problem, but generically it can
happen and this solution does not cover it.
The movement of links is provided to userspace, and userspace needs to
be smart about what it wants to do.
Alternatively, you could provide an API for your interface manager --
whatever is handling the link movement. It know where it is moving
interfaces. You could have it respond to inquiries of where device X is
at any given moment.
^ permalink raw reply
* Re: Containers and checkpoint/restart micro-conference at LPC2018
From: Christian Brauner @ 2018-09-09 1:31 UTC (permalink / raw)
To: Amir Goldstein
Cc: Stephane Graber, containers, Miklos Szeredi, Netdev, overlayfs,
lxc-users, LSM List, lxc-devel, linux-fsdevel
In-Reply-To: <CAOQ4uxiQEpofdS97kxnii8LtVW2QiKAGvjjaH0Px-Bj3eHVCFA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3905 bytes --]
On Sat, Sep 08, 2018 at 10:41:41AM +0300, Amir Goldstein wrote:
> On Sat, Sep 8, 2018 at 8:00 AM Stéphane Graber <stgraber@ubuntu.com> wrote:
> >
> > On Mon, Aug 13, 2018 at 12:10:15PM -0400, Stéphane Graber wrote:
> > > Hello,
> > >
> > > This year's edition of the Linux Plumbers Conference will once again
> > > have a containers micro-conference but this time around we'll have twice
> > > the usual amount of time and will include the content that would
> > > traditionally go into the checkpoint/restore micro-conference.
> > >
> > > LPC2018 will be held in Vancouver, Canada from the 13th to the 15th of
> > > November, co-located with the Linux Kernel Summit.
> > >
> > >
> > > We're looking for discussion topics around kernel work related to
> > > containers and namespacing, resource control, access control,
> > > checkpoint/restore of kernel structures, filesystem/mount handling for
> > > containers and any related userspace work.
> > >
> > >
> > > The format of the event will mostly be discussions where someone
> > > introduces a given topic/problem and it then gets discussed for 20-30min
> > > before moving on to something else. There will also be limited room for
> > > short demos of recent work with shorter 15min slots.
> > >
> > >
> > > Details can be found here:
> > >
> > > https://discuss.linuxcontainers.org/t/containers-micro-conference-at-linux-plumbers-2018/2417
> > >
> > >
> > > Looking forward to seeing you in Vancouver!
> >
> > Hello,
> >
> > We've added an extra week to the CFP, new deadline is Friday 14th of September.
> >
> > If you were thinking about sending something bug then forgot or just
> > missed the deadline, now is your chance to send it!
> >
>
> [cc: overlayfs developers]
>
> Hi Stéphane!
Hey Amir,
I'm one of the co-organizers of the microconf.
>
> I am not planing to travel to LPC this year, so this is more of an FYI than
> a CFP, but maybe another overlayfs developer can pick up this glove??
Sure, that would be great.
>
> For the past two years I have participated in the effort to fix overlayfs
> "non-standard" behavior:
> https://github.com/amir73il/overlayfs/wiki/Overlayfs-non-standard-behavior
Yes, this is an issue that we were aware of for a long time and it
something that has made overlayfs somewhat more difficult to use than it
should be.
>
> Allegedly, this effort went underway to improve the experience of overlayfs
> users, who are mostly applications running inside containers. For backward
> compatibility reasons, container runtimes will need to opt-in for fixing some
> of the legacy behavior.
>
> In reality, I have seen very little cross list interaction between linux-unionfs
> and containers mailing lists. The only interaction I recall in the
> past two years
> ended up in a fix in overlayfs to require opt-in for fixing yet another backward
> compatible bad behavior, although docker did follow up shortly after to fix
> bad practice in container runtime:
> https://github.com/moby/moby/issues/34672
>
> So the questions I would like to relay to the micro-conf participants w.r.t the
> new opt-in overlayfs behavior:
> 1. Did you know?
I personally did not know about the new opt-in behavior. More reason to
give a talk! :)
> 2. Do you care?
Yes, we do care. However - speaking as LXC upstream now - we have
recently focused on getting shiftfs to work rather than overlayfs.
We are more than happy to have a overlayfs talk at the microconf. If
someone were to talk about:
- What non-standard behavior has already been fixed?
- How has it been fixed?
- What non-standard behavior still needs to be fixed?
- Outstanding problems that either still need a solution or
are solved but one would like feedback on the implementation. This way
we can have a good discussion.
Thanks!
Christian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH net 00/13] cls_u32 cleanups and fixes.
From: Al Viro @ 2018-09-09 1:31 UTC (permalink / raw)
To: netdev; +Cc: Jamal Hadi Salim, Cong Wang, Jiri Pirko
From: Al Viro <viro@zeniv.linux.org.uk>
A series of net/sched/cls_u32.c cleanups and fixes:
1) fix hnode refcounting. Refcounting for tc_u_hnode is broken;
it's not hard to trigger oopsen (including one inside an interrupt handler,
with resulting panic) as well as memory corruption. Definitely -stable
fodder.
2) mark root hnode explicitly. Consistent errors on attempts to
delete root hnodes. Less serious than 1/13.
3) disallow linking to root hnode. Prohibit creating links to
root hnodes; not critical (nothing actually breaks if we do allow those),
but gets rid of surprising cases.
4) make sure that divisor is a power of 2. Missing validation -
divisor is documented as power of 2, but that's not actually enforced.
Results are moderately bogus (i.e. the kernel doesn't break), but rather
surprising.
Those are fixes, or at least can be argued to be such. The rest are pure
cleanups:
5) get rid of unused argument of u32_destroy_key()
6) get rid of tc_u_knode ->tp
7) get rid of tc_u_common ->rcu
Eliminate some unused fields.
8) clean tc_u_common hashtable.
Hash lookups are best done with minimum of calculations per chain
element - comparing the field in each candidate with f(key) where
f() is actually a pure function is not nice, especially when
compiler doesn't see f() as such... Better calculate f(key) once,
especially since we need its value to choose the hash chain in
the first place.
9) pass tc_u_common to u32_set_parms() instead of tc_u_hnode
10) the tp_c argument of u32_set_parms() is always tp->data
11) get rid of hnode ->tp_c and tp_c argument of u32_set_parms()
Massage that ends with getting rid of a redundant field.
12) keep track of knodes count in tc_u_common
13) simplify the hell out u32_delete() emptiness check
Checking if a filter needs to be killed after u32_delete() can be
done much easier - the test is equivalent to "filter doesn't
have ->data shared with anyone else and it has no knodes left
in it" and keeping track of the number of knodes is trivial.
^ permalink raw reply
* [PATCH net 02/13] net: sched: cls_u32: mark root hnode explicitly
From: Al Viro @ 2018-09-09 1:31 UTC (permalink / raw)
To: netdev; +Cc: Jamal Hadi Salim, Cong Wang, Jiri Pirko
In-Reply-To: <20180909013132.3222-1-viro@ZenIV.linux.org.uk>
From: Al Viro <viro@zeniv.linux.org.uk>
... and produce consistent error on attempt to delete such.
Existing check in u32_delete() is inconsistent - after
tc qdisc add dev eth0 ingress
tc filter add dev eth0 parent ffff: protocol ip prio 100 handle 1: u32 divisor 1
tc filter add dev eth0 parent ffff: protocol ip prio 200 handle 2: u32 divisor 1
both
tc filter delete dev eth0 parent ffff: protocol ip prio 100 handle 801: u32
and
tc filter delete dev eth0 parent ffff: protocol ip prio 100 handle 800: u32
will fail (at least with refcounting fixes), but the former will complain
about an attempt to remove a busy table, while the latter will recognize
it as root and yield "Not allowed to delete root node" instead.
The problem with the existing check is that several tcf_proto instances might
share the same tp->data and handle-to-hnode lookup will be the same for all
of them. So comparing an hnode to be deleted with tp->root won't catch the
case when one tp is used to try deleting the root of another. Solution is
trivial - mark the root hnodes explicitly upon allocation and check for that.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
net/sched/cls_u32.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index b2c3406a2cf2..c4782aa808c7 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -84,6 +84,7 @@ struct tc_u_hnode {
int refcnt;
unsigned int divisor;
struct idr handle_idr;
+ bool is_root;
struct rcu_head rcu;
u32 flags;
/* The 'ht' field MUST be the last field in structure to allow for
@@ -377,6 +378,7 @@ static int u32_init(struct tcf_proto *tp)
root_ht->refcnt++;
root_ht->handle = tp_c ? gen_new_htid(tp_c, root_ht) : 0x80000000;
root_ht->prio = tp->prio;
+ root_ht->is_root = true;
idr_init(&root_ht->handle_idr);
if (tp_c == NULL) {
@@ -693,7 +695,7 @@ static int u32_delete(struct tcf_proto *tp, void *arg, bool *last,
goto out;
}
- if (root_ht == ht) {
+ if (ht->is_root) {
NL_SET_ERR_MSG_MOD(extack, "Not allowed to delete root node");
return -EINVAL;
}
--
2.11.0
^ permalink raw reply related
* [PATCH net 03/13] net: sched: cls_u32: disallow linking to root hnode
From: Al Viro @ 2018-09-09 1:31 UTC (permalink / raw)
To: netdev; +Cc: Jamal Hadi Salim, Cong Wang, Jiri Pirko
In-Reply-To: <20180909013132.3222-1-viro@ZenIV.linux.org.uk>
From: Al Viro <viro@zeniv.linux.org.uk>
Operation makes no sense. Nothing will actually break if we do so
(depth limit in u32_classify() will prevent infinite loops), but
according to maintainers it's best prohibited outright.
NOTE: doing so guarantees that u32_destroy() will trigger the call
of u32_destroy_hnode(); we might want to make that unconditional.
Test:
tc qdisc add dev eth0 ingress
tc filter add dev eth0 parent ffff: protocol ip prio 100 u32 \
link 800: offset at 0 mask 0f00 shift 6 plus 0 eat match ip protocol 6 ff
should fail with
Error: cls_u32: Not linking to root node
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
net/sched/cls_u32.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index c4782aa808c7..72459b09d910 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -797,6 +797,10 @@ static int u32_set_parms(struct net *net, struct tcf_proto *tp,
NL_SET_ERR_MSG_MOD(extack, "Link hash table not found");
return -EINVAL;
}
+ if (ht_down->is_root) {
+ NL_SET_ERR_MSG_MOD(extack, "Not linking to root node");
+ return -EINVAL;
+ }
ht_down->refcnt++;
}
--
2.11.0
^ permalink raw reply related
* [PATCH net 04/13] net: sched: cls_u32: make sure that divisor is a power of 2
From: Al Viro @ 2018-09-09 1:31 UTC (permalink / raw)
To: netdev; +Cc: Jamal Hadi Salim, Cong Wang, Jiri Pirko
In-Reply-To: <20180909013132.3222-1-viro@ZenIV.linux.org.uk>
From: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
net/sched/cls_u32.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index 72459b09d910..d9923d474b65 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -994,7 +994,11 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
if (tb[TCA_U32_DIVISOR]) {
unsigned int divisor = nla_get_u32(tb[TCA_U32_DIVISOR]);
- if (--divisor > 0x100) {
+ if (!is_power_of_2(divisor)) {
+ NL_SET_ERR_MSG_MOD(extack, "Divisor is not a power of 2");
+ return -EINVAL;
+ }
+ if (divisor-- > 0x100) {
NL_SET_ERR_MSG_MOD(extack, "Exceeded maximum 256 hash buckets");
return -EINVAL;
}
--
2.11.0
^ permalink raw reply related
* [PATCH net 05/13] net: sched: cls_u32: get rid of unused argument of u32_destroy_key()
From: Al Viro @ 2018-09-09 1:31 UTC (permalink / raw)
To: netdev; +Cc: Jamal Hadi Salim, Cong Wang, Jiri Pirko
In-Reply-To: <20180909013132.3222-1-viro@ZenIV.linux.org.uk>
From: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
net/sched/cls_u32.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index d9923d474b65..d11862823911 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -406,8 +406,7 @@ static int u32_init(struct tcf_proto *tp)
return 0;
}
-static int u32_destroy_key(struct tcf_proto *tp, struct tc_u_knode *n,
- bool free_pf)
+static int u32_destroy_key(struct tc_u_knode *n, bool free_pf)
{
struct tc_u_hnode *ht = rtnl_dereference(n->ht_down);
@@ -441,7 +440,7 @@ static void u32_delete_key_work(struct work_struct *work)
struct tc_u_knode,
rwork);
rtnl_lock();
- u32_destroy_key(key->tp, key, false);
+ u32_destroy_key(key, false);
rtnl_unlock();
}
@@ -458,7 +457,7 @@ static void u32_delete_key_freepf_work(struct work_struct *work)
struct tc_u_knode,
rwork);
rtnl_lock();
- u32_destroy_key(key->tp, key, true);
+ u32_destroy_key(key, true);
rtnl_unlock();
}
@@ -601,7 +600,7 @@ static void u32_clear_hnode(struct tcf_proto *tp, struct tc_u_hnode *ht,
if (tcf_exts_get_net(&n->exts))
tcf_queue_work(&n->rwork, u32_delete_key_freepf_work);
else
- u32_destroy_key(n->tp, n, true);
+ u32_destroy_key(n, true);
}
}
}
@@ -971,13 +970,13 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
tca[TCA_RATE], ovr, extack);
if (err) {
- u32_destroy_key(tp, new, false);
+ u32_destroy_key(new, false);
return err;
}
err = u32_replace_hw_knode(tp, new, flags, extack);
if (err) {
- u32_destroy_key(tp, new, false);
+ u32_destroy_key(new, false);
return err;
}
--
2.11.0
^ permalink raw reply related
* [PATCH net 06/13] net: sched: cls_u32: get rid of tc_u_knode ->tp
From: Al Viro @ 2018-09-09 1:31 UTC (permalink / raw)
To: netdev; +Cc: Jamal Hadi Salim, Cong Wang, Jiri Pirko
In-Reply-To: <20180909013132.3222-1-viro@ZenIV.linux.org.uk>
From: Al Viro <viro@zeniv.linux.org.uk>
not used anymore
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
net/sched/cls_u32.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index d11862823911..281ac954511c 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -68,7 +68,6 @@ struct tc_u_knode {
u32 mask;
u32 __percpu *pcpu_success;
#endif
- struct tcf_proto *tp;
struct rcu_work rwork;
/* The 'sel' field MUST be the last field in structure to allow for
* tc_u32_keys allocated at end of structure.
@@ -896,7 +895,6 @@ static struct tc_u_knode *u32_init_knode(struct tcf_proto *tp,
/* Similarly success statistics must be moved as pointers */
new->pcpu_success = n->pcpu_success;
#endif
- new->tp = tp;
memcpy(&new->sel, s, sizeof(*s) + s->nkeys*sizeof(struct tc_u32_key));
if (tcf_exts_init(&new->exts, TCA_U32_ACT, TCA_U32_POLICE)) {
@@ -1112,7 +1110,6 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
n->handle = handle;
n->fshift = s->hmask ? ffs(ntohl(s->hmask)) - 1 : 0;
n->flags = flags;
- n->tp = tp;
err = tcf_exts_init(&n->exts, TCA_U32_ACT, TCA_U32_POLICE);
if (err < 0)
--
2.11.0
^ permalink raw reply related
* [PATCH net 07/13] net: sched: cls_u32: get rid of tc_u_common ->rcu
From: Al Viro @ 2018-09-09 1:31 UTC (permalink / raw)
To: netdev; +Cc: Jamal Hadi Salim, Cong Wang, Jiri Pirko
In-Reply-To: <20180909013132.3222-1-viro@ZenIV.linux.org.uk>
From: Al Viro <viro@zeniv.linux.org.uk>
unused
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
net/sched/cls_u32.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index 281ac954511c..1bfbfcab7260 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -98,7 +98,6 @@ struct tc_u_common {
int refcnt;
struct idr handle_idr;
struct hlist_node hnode;
- struct rcu_head rcu;
};
static inline unsigned int u32_hash_fold(__be32 key,
--
2.11.0
^ permalink raw reply related
* [PATCH net 08/13] net: sched: cls_u32: clean tc_u_common hashtable
From: Al Viro @ 2018-09-09 1:31 UTC (permalink / raw)
To: netdev; +Cc: Jamal Hadi Salim, Cong Wang, Jiri Pirko
In-Reply-To: <20180909013132.3222-1-viro@ZenIV.linux.org.uk>
From: Al Viro <viro@zeniv.linux.org.uk>
* calculate key *once*, not for each hash chain element
* let tc_u_hash() return the pointer to chain head rather than index -
callers are cleaner that way.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
net/sched/cls_u32.c | 24 +++++++++---------------
1 file changed, 9 insertions(+), 15 deletions(-)
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index 1bfbfcab7260..af05113c1212 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -343,19 +343,16 @@ static void *tc_u_common_ptr(const struct tcf_proto *tp)
return block->q;
}
-static unsigned int tc_u_hash(const struct tcf_proto *tp)
+static struct hlist_head *tc_u_hash(void *key)
{
- return hash_ptr(tc_u_common_ptr(tp), U32_HASH_SHIFT);
+ return tc_u_common_hash + hash_ptr(key, U32_HASH_SHIFT);
}
-static struct tc_u_common *tc_u_common_find(const struct tcf_proto *tp)
+static struct tc_u_common *tc_u_common_find(void *key)
{
struct tc_u_common *tc;
- unsigned int h;
-
- h = tc_u_hash(tp);
- hlist_for_each_entry(tc, &tc_u_common_hash[h], hnode) {
- if (tc->ptr == tc_u_common_ptr(tp))
+ hlist_for_each_entry(tc, tc_u_hash(key), hnode) {
+ if (tc->ptr == key)
return tc;
}
return NULL;
@@ -364,10 +361,8 @@ static struct tc_u_common *tc_u_common_find(const struct tcf_proto *tp)
static int u32_init(struct tcf_proto *tp)
{
struct tc_u_hnode *root_ht;
- struct tc_u_common *tp_c;
- unsigned int h;
-
- tp_c = tc_u_common_find(tp);
+ void *key = tc_u_common_ptr(tp);
+ struct tc_u_common *tp_c = tc_u_common_find(key);
root_ht = kzalloc(sizeof(*root_ht), GFP_KERNEL);
if (root_ht == NULL)
@@ -385,12 +380,11 @@ static int u32_init(struct tcf_proto *tp)
kfree(root_ht);
return -ENOBUFS;
}
- tp_c->ptr = tc_u_common_ptr(tp);
+ tp_c->ptr = key;
INIT_HLIST_NODE(&tp_c->hnode);
idr_init(&tp_c->handle_idr);
- h = tc_u_hash(tp);
- hlist_add_head(&tp_c->hnode, &tc_u_common_hash[h]);
+ hlist_add_head(&tp_c->hnode, tc_u_hash(key));
}
tp_c->refcnt++;
--
2.11.0
^ permalink raw reply related
* [PATCH net 09/13] net: sched: cls_u32: pass tc_u_common to u32_set_parms() instead of tc_u_hnode
From: Al Viro @ 2018-09-09 1:31 UTC (permalink / raw)
To: netdev; +Cc: Jamal Hadi Salim, Cong Wang, Jiri Pirko
In-Reply-To: <20180909013132.3222-1-viro@ZenIV.linux.org.uk>
From: Al Viro <viro@zeniv.linux.org.uk>
the only thing we used ht for was ht->tp_c and callers can get that
without going through ->tp_c at all; start with lifting that into
the callers, next commits will massage those, eventually removing
->tp_c altogether.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
net/sched/cls_u32.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index af05113c1212..221ce532b241 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -761,7 +761,7 @@ static const struct nla_policy u32_policy[TCA_U32_MAX + 1] = {
};
static int u32_set_parms(struct net *net, struct tcf_proto *tp,
- unsigned long base, struct tc_u_hnode *ht,
+ unsigned long base, struct tc_u_common *tp_c,
struct tc_u_knode *n, struct nlattr **tb,
struct nlattr *est, bool ovr,
struct netlink_ext_ack *extack)
@@ -782,7 +782,7 @@ static int u32_set_parms(struct net *net, struct tcf_proto *tp,
}
if (handle) {
- ht_down = u32_lookup_ht(ht->tp_c, handle);
+ ht_down = u32_lookup_ht(tp_c, handle);
if (!ht_down) {
NL_SET_ERR_MSG_MOD(extack, "Link hash table not found");
@@ -957,7 +957,7 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
return -ENOMEM;
err = u32_set_parms(net, tp, base,
- rtnl_dereference(n->ht_up), new, tb,
+ rtnl_dereference(n->ht_up)->tp_c, new, tb,
tca[TCA_RATE], ovr, extack);
if (err) {
@@ -1124,7 +1124,7 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
}
#endif
- err = u32_set_parms(net, tp, base, ht, n, tb, tca[TCA_RATE], ovr,
+ err = u32_set_parms(net, tp, base, ht->tp_c, n, tb, tca[TCA_RATE], ovr,
extack);
if (err == 0) {
struct tc_u_knode __rcu **ins;
--
2.11.0
^ permalink raw reply related
* [PATCH net 10/13] net: sched: cls_u32: the tp_c argument of u32_set_parms() is always tp->data
From: Al Viro @ 2018-09-09 1:31 UTC (permalink / raw)
To: netdev; +Cc: Jamal Hadi Salim, Cong Wang, Jiri Pirko
In-Reply-To: <20180909013132.3222-1-viro@ZenIV.linux.org.uk>
From: Al Viro <viro@zeniv.linux.org.uk>
It must be tc_u_common associated with that tp (i.e. tp->data).
Proof:
* both ->ht_up and ->tp_c are assign-once
* ->tp_c of anything inserted into tp_c->hlist is tp_c
* hnodes never get reinserted into the lists or moved
between those, so anything found by u32_lookup_ht(tp->data, ...)
will have ->tp_c equal to tp->data.
* tp->root->tp_c == tp->data.
* ->ht_up of anything inserted into hnode->ht[...] is
equal to hnode.
* knodes never get reinserted into hash chains or moved
between those, so anything returned by u32_lookup_key(ht, ...)
will have ->ht_up equal to ht.
* any knode returned by u32_get(tp, ...) will have ->ht_up->tp_c
point to tp->data
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
net/sched/cls_u32.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index 221ce532b241..12757e3ec8d8 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -956,8 +956,7 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
if (!new)
return -ENOMEM;
- err = u32_set_parms(net, tp, base,
- rtnl_dereference(n->ht_up)->tp_c, new, tb,
+ err = u32_set_parms(net, tp, base, tp_c, new, tb,
tca[TCA_RATE], ovr, extack);
if (err) {
@@ -1124,7 +1123,7 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
}
#endif
- err = u32_set_parms(net, tp, base, ht->tp_c, n, tb, tca[TCA_RATE], ovr,
+ err = u32_set_parms(net, tp, base, tp_c, n, tb, tca[TCA_RATE], ovr,
extack);
if (err == 0) {
struct tc_u_knode __rcu **ins;
--
2.11.0
^ permalink raw reply related
* [PATCH net 11/13] net: sched: cls_u32: get rid of hnode ->tp_c and tp_c argument of u32_set_parms()
From: Al Viro @ 2018-09-09 1:31 UTC (permalink / raw)
To: netdev; +Cc: Jamal Hadi Salim, Cong Wang, Jiri Pirko
In-Reply-To: <20180909013132.3222-1-viro@ZenIV.linux.org.uk>
From: Al Viro <viro@zeniv.linux.org.uk>
the latter is redundant, the former - never read...
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
net/sched/cls_u32.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index 12757e3ec8d8..f6bb3885598d 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -79,7 +79,6 @@ struct tc_u_hnode {
struct tc_u_hnode __rcu *next;
u32 handle;
u32 prio;
- struct tc_u_common *tp_c;
int refcnt;
unsigned int divisor;
struct idr handle_idr;
@@ -390,7 +389,6 @@ static int u32_init(struct tcf_proto *tp)
tp_c->refcnt++;
RCU_INIT_POINTER(root_ht->next, tp_c->hlist);
rcu_assign_pointer(tp_c->hlist, root_ht);
- root_ht->tp_c = tp_c;
root_ht->refcnt++;
rcu_assign_pointer(tp->root, root_ht);
@@ -761,7 +759,7 @@ static const struct nla_policy u32_policy[TCA_U32_MAX + 1] = {
};
static int u32_set_parms(struct net *net, struct tcf_proto *tp,
- unsigned long base, struct tc_u_common *tp_c,
+ unsigned long base,
struct tc_u_knode *n, struct nlattr **tb,
struct nlattr *est, bool ovr,
struct netlink_ext_ack *extack)
@@ -782,7 +780,7 @@ static int u32_set_parms(struct net *net, struct tcf_proto *tp,
}
if (handle) {
- ht_down = u32_lookup_ht(tp_c, handle);
+ ht_down = u32_lookup_ht(tp->data, handle);
if (!ht_down) {
NL_SET_ERR_MSG_MOD(extack, "Link hash table not found");
@@ -956,7 +954,7 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
if (!new)
return -ENOMEM;
- err = u32_set_parms(net, tp, base, tp_c, new, tb,
+ err = u32_set_parms(net, tp, base, new, tb,
tca[TCA_RATE], ovr, extack);
if (err) {
@@ -1012,7 +1010,6 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
return err;
}
}
- ht->tp_c = tp_c;
ht->refcnt = 1;
ht->divisor = divisor;
ht->handle = handle;
@@ -1123,8 +1120,7 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
}
#endif
- err = u32_set_parms(net, tp, base, tp_c, n, tb, tca[TCA_RATE], ovr,
- extack);
+ err = u32_set_parms(net, tp, base, n, tb, tca[TCA_RATE], ovr, extack);
if (err == 0) {
struct tc_u_knode __rcu **ins;
struct tc_u_knode *pins;
--
2.11.0
^ permalink raw reply related
* [PATCH net 12/13] net: sched: cls_u32: keep track of knodes count in tc_u_common
From: Al Viro @ 2018-09-09 1:31 UTC (permalink / raw)
To: netdev; +Cc: Jamal Hadi Salim, Cong Wang, Jiri Pirko
In-Reply-To: <20180909013132.3222-1-viro@ZenIV.linux.org.uk>
From: Al Viro <viro@zeniv.linux.org.uk>
allows to simplify u32_delete() considerably
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
net/sched/cls_u32.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index f6bb3885598d..86cbe4f5800e 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -97,6 +97,7 @@ struct tc_u_common {
int refcnt;
struct idr handle_idr;
struct hlist_node hnode;
+ long knodes;
};
static inline unsigned int u32_hash_fold(__be32 key,
@@ -453,6 +454,7 @@ static void u32_delete_key_freepf_work(struct work_struct *work)
static int u32_delete_key(struct tcf_proto *tp, struct tc_u_knode *key)
{
+ struct tc_u_common *tp_c = tp->data;
struct tc_u_knode __rcu **kp;
struct tc_u_knode *pkp;
struct tc_u_hnode *ht = rtnl_dereference(key->ht_up);
@@ -463,6 +465,7 @@ static int u32_delete_key(struct tcf_proto *tp, struct tc_u_knode *key)
kp = &pkp->next, pkp = rtnl_dereference(*kp)) {
if (pkp == key) {
RCU_INIT_POINTER(*kp, key->next);
+ tp_c->knodes--;
tcf_unbind_filter(tp, &key->res);
idr_remove(&ht->handle_idr, key->handle);
@@ -577,6 +580,7 @@ static int u32_replace_hw_knode(struct tcf_proto *tp, struct tc_u_knode *n,
static void u32_clear_hnode(struct tcf_proto *tp, struct tc_u_hnode *ht,
struct netlink_ext_ack *extack)
{
+ struct tc_u_common *tp_c = tp->data;
struct tc_u_knode *n;
unsigned int h;
@@ -584,6 +588,7 @@ static void u32_clear_hnode(struct tcf_proto *tp, struct tc_u_hnode *ht,
while ((n = rtnl_dereference(ht->ht[h])) != NULL) {
RCU_INIT_POINTER(ht->ht[h],
rtnl_dereference(n->next));
+ tp_c->knodes--;
tcf_unbind_filter(tp, &n->res);
u32_remove_hw_knode(tp, n, extack);
idr_remove(&ht->handle_idr, n->handle);
@@ -1140,6 +1145,7 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
RCU_INIT_POINTER(n->next, pins);
rcu_assign_pointer(*ins, n);
+ tp_c->knodes++;
*arg = n;
return 0;
}
--
2.11.0
^ permalink raw reply related
* [PATCH net 13/13] net: sched: cls_u32: simplify the hell out u32_delete() emptiness check
From: Al Viro @ 2018-09-09 1:31 UTC (permalink / raw)
To: netdev; +Cc: Jamal Hadi Salim, Cong Wang, Jiri Pirko
In-Reply-To: <20180909013132.3222-1-viro@ZenIV.linux.org.uk>
From: Al Viro <viro@zeniv.linux.org.uk>
Now that we have the knode count, we can instantly check if
any hnodes are non-empty. And that kills the check for extra
references to root hnode - those could happen only if there was
a knode to carry such a link.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
net/sched/cls_u32.c | 48 +-----------------------------------------------
1 file changed, 1 insertion(+), 47 deletions(-)
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index 86cbe4f5800e..ef786ec24843 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -628,17 +628,6 @@ static int u32_destroy_hnode(struct tcf_proto *tp, struct tc_u_hnode *ht,
return -ENOENT;
}
-static bool ht_empty(struct tc_u_hnode *ht)
-{
- unsigned int h;
-
- for (h = 0; h <= ht->divisor; h++)
- if (rcu_access_pointer(ht->ht[h]))
- return false;
-
- return true;
-}
-
static void u32_destroy(struct tcf_proto *tp, struct netlink_ext_ack *extack)
{
struct tc_u_common *tp_c = tp->data;
@@ -676,13 +665,9 @@ static int u32_delete(struct tcf_proto *tp, void *arg, bool *last,
struct netlink_ext_ack *extack)
{
struct tc_u_hnode *ht = arg;
- struct tc_u_hnode *root_ht = rtnl_dereference(tp->root);
struct tc_u_common *tp_c = tp->data;
int ret = 0;
- if (ht == NULL)
- goto out;
-
if (TC_U32_KEY(ht->handle)) {
u32_remove_hw_knode(tp, (struct tc_u_knode *)ht, extack);
ret = u32_delete_key(tp, (struct tc_u_knode *)ht);
@@ -702,38 +687,7 @@ static int u32_delete(struct tcf_proto *tp, void *arg, bool *last,
}
out:
- *last = true;
- if (root_ht) {
- if (root_ht->refcnt > 2) {
- *last = false;
- goto ret;
- }
- if (root_ht->refcnt == 2) {
- if (!ht_empty(root_ht)) {
- *last = false;
- goto ret;
- }
- }
- }
-
- if (tp_c->refcnt > 1) {
- *last = false;
- goto ret;
- }
-
- if (tp_c->refcnt == 1) {
- struct tc_u_hnode *ht;
-
- for (ht = rtnl_dereference(tp_c->hlist);
- ht;
- ht = rtnl_dereference(ht->next))
- if (!ht_empty(ht)) {
- *last = false;
- break;
- }
- }
-
-ret:
+ *last = tp_c->refcnt == 1 && tp_c->knodes == 0;
return ret;
}
--
2.11.0
^ permalink raw reply related
* Re: [PATCH net] ipv6: use rt6_info members when dst is set in rt6_fill_node
From: David Ahern @ 2018-09-09 1:44 UTC (permalink / raw)
To: Xin Long, network dev; +Cc: davem, Roopa Prabhu
In-Reply-To: <840f0c017c670ae9cc8fd8330ca24fcca1207918.1536398641.git.lucien.xin@gmail.com>
On 9/8/18 3:24 AM, Xin Long wrote:
> In inet6_rtm_getroute, since Commit 93531c674315 ("net/ipv6: separate
> handling of FIB entries from dst based routes"), it has used rt->from
> to dump route info instead of rt.
>
> However for some route like cache, its information is not the same as
> that of the 'from' one. It caused 'ip -6 route get' to dump the wrong
> route information.
>
> In Jianlin's testing, the output information even lost the expiration
> time for a pmtu route cache due to the wrong fib6_flags.
you are right about the flags ...
>
> So change to use rt6_info members when it tries to dump a route entry
> without fibmatch set.
but not the src, dst and prefsrc.
>
> Note that we will fix the gw/nh dump in another patch.
And only the gateway can change do to a redirect and redirects do not
change the device - only the gateway.
Let's do both changes in a single patch.
>
> Fixes: 93531c674315 ("net/ipv6: separate handling of FIB entries from dst based routes")
> Reported-by: Jianlin Shi <jishi@redhat.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
> net/ipv6/route.c | 39 ++++++++++++++++++++++++++-------------
> 1 file changed, 26 insertions(+), 13 deletions(-)
>
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 18e00ce..e554922 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -4670,20 +4670,33 @@ static int rt6_fill_node(struct net *net, struct sk_buff *skb,
> int iif, int type, u32 portid, u32 seq,
> unsigned int flags)
> {
> - struct rtmsg *rtm;
> + struct rt6key *fib6_prefsrc, *fib6_dst, *fib6_src;
> + struct rt6_info *rt6 = (struct rt6_info *)dst;
> + u32 *pmetrics, table, fib6_flags;
> struct nlmsghdr *nlh;
> + struct rtmsg *rtm;
> long expires = 0;
> - u32 *pmetrics;
> - u32 table;
>
> nlh = nlmsg_put(skb, portid, seq, type, sizeof(*rtm), flags);
> if (!nlh)
> return -EMSGSIZE;
>
> + if (rt6) {
> + fib6_dst = &rt6->rt6i_dst;
> + fib6_src = &rt6->rt6i_src;
> + fib6_flags = rt6->rt6i_flags;
> + fib6_prefsrc = &rt6->rt6i_prefsrc;
> + } else {
> + fib6_dst = &rt->fib6_dst;
> + fib6_src = &rt->fib6_src;
> + fib6_flags = rt->fib6_flags;
> + fib6_prefsrc = &rt->fib6_prefsrc;
> + }
Unless I am missing something at the moment, an rt6_info can only have
the same dst, src and prefsrc as the fib6_info on which it is based.
Thus, only the flags is needed above. That simplifies this patch a lot.
^ permalink raw reply
* Re: [PATCH net-next v8 5/7] net: vhost: introduce bitmap for vhost_poll
From: Tonghao Zhang @ 2018-09-09 10:58 UTC (permalink / raw)
To: jasowang; +Cc: Linux Kernel Network Developers, virtualization, mst
In-Reply-To: <24f10821-a07e-e0cd-0266-b33856e7d88a@redhat.com>
On Tue, Aug 21, 2018 at 8:45 AM Jason Wang <jasowang@redhat.com> wrote:
>
>
>
> On 2018年08月19日 20:11, xiangxia.m.yue@gmail.com wrote:
> > From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> >
> > The bitmap of vhost_dev can help us to check if the
> > specified poll is scheduled. This patch will be used
> > for next two patches.
> >
> > Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> > ---
> > drivers/vhost/net.c | 11 +++++++++--
> > drivers/vhost/vhost.c | 17 +++++++++++++++--
> > drivers/vhost/vhost.h | 7 ++++++-
> > 3 files changed, 30 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> > index 1eff72d..23d7ffc 100644
> > --- a/drivers/vhost/net.c
> > +++ b/drivers/vhost/net.c
> > @@ -1135,8 +1135,15 @@ static int vhost_net_open(struct inode *inode, struct file *f)
> > }
> > vhost_dev_init(dev, vqs, VHOST_NET_VQ_MAX);
> >
> > - vhost_poll_init(n->poll + VHOST_NET_VQ_TX, handle_tx_net, EPOLLOUT, dev);
> > - vhost_poll_init(n->poll + VHOST_NET_VQ_RX, handle_rx_net, EPOLLIN, dev);
> > + vhost_poll_init(n->poll + VHOST_NET_VQ_TX,
> > + handle_tx_net,
> > + VHOST_NET_VQ_TX,
> > + EPOLLOUT, dev);
> > +
> > + vhost_poll_init(n->poll + VHOST_NET_VQ_RX,
> > + handle_rx_net,
> > + VHOST_NET_VQ_RX,
> > + EPOLLIN, dev);
> >
> > f->private_data = n;
> >
> > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > index a1c06e7..dc88a60 100644
> > --- a/drivers/vhost/vhost.c
> > +++ b/drivers/vhost/vhost.c
> > @@ -186,7 +186,7 @@ void vhost_work_init(struct vhost_work *work, vhost_work_fn_t fn)
> >
> > /* Init poll structure */
> > void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn,
> > - __poll_t mask, struct vhost_dev *dev)
> > + __u8 poll_id, __poll_t mask, struct vhost_dev *dev)
> > {
> > init_waitqueue_func_entry(&poll->wait, vhost_poll_wakeup);
> > init_poll_funcptr(&poll->table, vhost_poll_func);
> > @@ -194,6 +194,7 @@ void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn,
> > poll->dev = dev;
> > poll->wqh = NULL;
> >
> > + poll->poll_id = poll_id;
> > vhost_work_init(&poll->work, fn);
> > }
> > EXPORT_SYMBOL_GPL(vhost_poll_init);
> > @@ -276,8 +277,16 @@ bool vhost_has_work(struct vhost_dev *dev)
> > }
> > EXPORT_SYMBOL_GPL(vhost_has_work);
> >
> > +bool vhost_has_work_pending(struct vhost_dev *dev, int poll_id)
> > +{
> > + return !llist_empty(&dev->work_list) &&
> > + test_bit(poll_id, dev->work_pending);
>
> I think we've already had something similar. E.g can we test
> VHOST_WORK_QUEUED instead?
done, thanks. I am busy two weeks ago, and now I focus on these patches.
> Thanks
>
> > +}
> > +EXPORT_SYMBOL_GPL(vhost_has_work_pending);
> > +
> > void vhost_poll_queue(struct vhost_poll *poll)
> > {
> > + set_bit(poll->poll_id, poll->dev->work_pending);
> > vhost_work_queue(poll->dev, &poll->work);
> > }
> > EXPORT_SYMBOL_GPL(vhost_poll_queue);
> > @@ -354,6 +363,7 @@ static int vhost_worker(void *data)
> > if (!node)
> > schedule();
> >
> > + bitmap_zero(dev->work_pending, VHOST_DEV_MAX_VQ);
> > node = llist_reverse_order(node);
> > /* make sure flag is seen after deletion */
> > smp_wmb();
> > @@ -420,6 +430,8 @@ void vhost_dev_init(struct vhost_dev *dev,
> > struct vhost_virtqueue *vq;
> > int i;
> >
> > + BUG_ON(nvqs > VHOST_DEV_MAX_VQ);
> > +
> > dev->vqs = vqs;
> > dev->nvqs = nvqs;
> > mutex_init(&dev->mutex);
> > @@ -428,6 +440,7 @@ void vhost_dev_init(struct vhost_dev *dev,
> > dev->iotlb = NULL;
> > dev->mm = NULL;
> > dev->worker = NULL;
> > + bitmap_zero(dev->work_pending, VHOST_DEV_MAX_VQ);
> > init_llist_head(&dev->work_list);
> > init_waitqueue_head(&dev->wait);
> > INIT_LIST_HEAD(&dev->read_list);
> > @@ -445,7 +458,7 @@ void vhost_dev_init(struct vhost_dev *dev,
> > vhost_vq_reset(dev, vq);
> > if (vq->handle_kick)
> > vhost_poll_init(&vq->poll, vq->handle_kick,
> > - EPOLLIN, dev);
> > + i, EPOLLIN, dev);
> > }
> > }
> > EXPORT_SYMBOL_GPL(vhost_dev_init);
> > diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> > index 6c844b9..60b6f6d 100644
> > --- a/drivers/vhost/vhost.h
> > +++ b/drivers/vhost/vhost.h
> > @@ -30,6 +30,7 @@ struct vhost_poll {
> > wait_queue_head_t *wqh;
> > wait_queue_entry_t wait;
> > struct vhost_work work;
> > + __u8 poll_id;
> > __poll_t mask;
> > struct vhost_dev *dev;
> > };
> > @@ -37,9 +38,10 @@ struct vhost_poll {
> > void vhost_work_init(struct vhost_work *work, vhost_work_fn_t fn);
> > void vhost_work_queue(struct vhost_dev *dev, struct vhost_work *work);
> > bool vhost_has_work(struct vhost_dev *dev);
> > +bool vhost_has_work_pending(struct vhost_dev *dev, int poll_id);
> >
> > void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn,
> > - __poll_t mask, struct vhost_dev *dev);
> > + __u8 id, __poll_t mask, struct vhost_dev *dev);
> > int vhost_poll_start(struct vhost_poll *poll, struct file *file);
> > void vhost_poll_stop(struct vhost_poll *poll);
> > void vhost_poll_flush(struct vhost_poll *poll);
> > @@ -152,6 +154,8 @@ struct vhost_msg_node {
> > struct list_head node;
> > };
> >
> > +#define VHOST_DEV_MAX_VQ 128
> > +
> > struct vhost_dev {
> > struct mm_struct *mm;
> > struct mutex mutex;
> > @@ -159,6 +163,7 @@ struct vhost_dev {
> > int nvqs;
> > struct eventfd_ctx *log_ctx;
> > struct llist_head work_list;
> > + DECLARE_BITMAP(work_pending, VHOST_DEV_MAX_VQ);
> > struct task_struct *worker;
> > struct vhost_umem *umem;
> > struct vhost_umem *iotlb;
>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH net-next v8 3/7] net: vhost: factor out busy polling logic to vhost_net_busy_poll()
From: Tonghao Zhang @ 2018-09-09 10:58 UTC (permalink / raw)
To: jasowang; +Cc: Linux Kernel Network Developers, virtualization, mst
In-Reply-To: <179340e9-f1cd-8378-1a01-61307546003c@redhat.com>
On Tue, Aug 21, 2018 at 11:15 AM Jason Wang <jasowang@redhat.com> wrote:
>
>
>
> On 2018年08月19日 20:11, xiangxia.m.yue@gmail.com wrote:
> > From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> >
> > Factor out generic busy polling logic and will be
> > used for in tx path in the next patch. And with the patch,
> > qemu can set differently the busyloop_timeout for rx queue.
> >
> > To avoid duplicate codes, introduce the helper functions:
> > * sock_has_rx_data(changed from sk_has_rx_data)
> > * vhost_net_busy_poll_try_queue
> >
> > Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> > ---
> > drivers/vhost/net.c | 111 +++++++++++++++++++++++++++++++++-------------------
> > 1 file changed, 71 insertions(+), 40 deletions(-)
> >
> > diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> > index 32c1b52..453c061 100644
> > --- a/drivers/vhost/net.c
> > +++ b/drivers/vhost/net.c
> > @@ -440,6 +440,75 @@ static void vhost_net_signal_used(struct vhost_net_virtqueue *nvq)
> > nvq->done_idx = 0;
> > }
> >
> > +static int sock_has_rx_data(struct socket *sock)
> > +{
> > + if (unlikely(!sock))
> > + return 0;
> > +
> > + if (sock->ops->peek_len)
> > + return sock->ops->peek_len(sock);
> > +
> > + return skb_queue_empty(&sock->sk->sk_receive_queue);
> > +}
> > +
> > +static void vhost_net_busy_poll_try_queue(struct vhost_net *net,
> > + struct vhost_virtqueue *vq)
> > +{
> > + if (!vhost_vq_avail_empty(&net->dev, vq)) {
> > + vhost_poll_queue(&vq->poll);
> > + } else if (unlikely(vhost_enable_notify(&net->dev, vq))) {
> > + vhost_disable_notify(&net->dev, vq);
> > + vhost_poll_queue(&vq->poll);
> > + }
> > +}
> > +
> > +static void vhost_net_busy_poll(struct vhost_net *net,
> > + struct vhost_virtqueue *rvq,
> > + struct vhost_virtqueue *tvq,
> > + bool *busyloop_intr,
> > + bool poll_rx)
> > +{
> > + unsigned long busyloop_timeout;
> > + unsigned long endtime;
> > + struct socket *sock;
> > + struct vhost_virtqueue *vq = poll_rx ? tvq : rvq;
> > +
> > + mutex_lock_nested(&vq->mutex, poll_rx ? VHOST_NET_VQ_TX: VHOST_NET_VQ_RX);
> > + vhost_disable_notify(&net->dev, vq);
> > + sock = rvq->private_data;
> > +
> > + busyloop_timeout = poll_rx ? rvq->busyloop_timeout:
> > + tvq->busyloop_timeout;
> > +
> > + preempt_disable();
> > + endtime = busy_clock() + busyloop_timeout;
> > +
> > + while (vhost_can_busy_poll(endtime)) {
> > + if (vhost_has_work(&net->dev)) {
> > + *busyloop_intr = true;
> > + break;
> > + }
> > +
> > + if ((sock_has_rx_data(sock) &&
> > + !vhost_vq_avail_empty(&net->dev, rvq)) ||
> > + !vhost_vq_avail_empty(&net->dev, tvq))
> > + break;
> > +
> > + cpu_relax();
> > + }
> > +
> > + preempt_enable();
> > +
> > + if (poll_rx)
> > + vhost_net_busy_poll_try_queue(net, tvq);
> > + else if (sock_has_rx_data(sock))
> > + vhost_net_busy_poll_try_queue(net, rvq);
>
> This could be simplified like:
>
> if (poll_rx || sock_has_rx_data(sock))
> vhost_net_busy_poll_try_queue(net, vq);
done, thanks
> Thanks
>
> > + else /* On tx here, sock has no rx data. */
> > + vhost_enable_notify(&net->dev, rvq);
> > +
> > + mutex_unlock(&vq->mutex);
> > +}
> > +
> > static int vhost_net_tx_get_vq_desc(struct vhost_net *net,
> > struct vhost_net_virtqueue *nvq,
> > unsigned int *out_num, unsigned int *in_num,
> > @@ -753,16 +822,6 @@ static int peek_head_len(struct vhost_net_virtqueue *rvq, struct sock *sk)
> > return len;
> > }
> >
> > -static int sk_has_rx_data(struct sock *sk)
> > -{
> > - struct socket *sock = sk->sk_socket;
> > -
> > - if (sock->ops->peek_len)
> > - return sock->ops->peek_len(sock);
> > -
> > - return skb_queue_empty(&sk->sk_receive_queue);
> > -}
> > -
> > static int vhost_net_rx_peek_head_len(struct vhost_net *net, struct sock *sk,
> > bool *busyloop_intr)
> > {
> > @@ -770,41 +829,13 @@ static int vhost_net_rx_peek_head_len(struct vhost_net *net, struct sock *sk,
> > struct vhost_net_virtqueue *tnvq = &net->vqs[VHOST_NET_VQ_TX];
> > struct vhost_virtqueue *rvq = &rnvq->vq;
> > struct vhost_virtqueue *tvq = &tnvq->vq;
> > - unsigned long uninitialized_var(endtime);
> > int len = peek_head_len(rnvq, sk);
> >
> > - if (!len && tvq->busyloop_timeout) {
> > + if (!len && rvq->busyloop_timeout) {
> > /* Flush batched heads first */
> > vhost_net_signal_used(rnvq);
> > /* Both tx vq and rx socket were polled here */
> > - mutex_lock_nested(&tvq->mutex, VHOST_NET_VQ_TX);
> > - vhost_disable_notify(&net->dev, tvq);
> > -
> > - preempt_disable();
> > - endtime = busy_clock() + tvq->busyloop_timeout;
> > -
> > - while (vhost_can_busy_poll(endtime)) {
> > - if (vhost_has_work(&net->dev)) {
> > - *busyloop_intr = true;
> > - break;
> > - }
> > - if ((sk_has_rx_data(sk) &&
> > - !vhost_vq_avail_empty(&net->dev, rvq)) ||
> > - !vhost_vq_avail_empty(&net->dev, tvq))
> > - break;
> > - cpu_relax();
> > - }
> > -
> > - preempt_enable();
> > -
> > - if (!vhost_vq_avail_empty(&net->dev, tvq)) {
> > - vhost_poll_queue(&tvq->poll);
> > - } else if (unlikely(vhost_enable_notify(&net->dev, tvq))) {
> > - vhost_disable_notify(&net->dev, tvq);
> > - vhost_poll_queue(&tvq->poll);
> > - }
> > -
> > - mutex_unlock(&tvq->mutex);
> > + vhost_net_busy_poll(net, rvq, tvq, busyloop_intr, true);
> >
> > len = peek_head_len(rnvq, sk);
> > }
>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH RFC 3/5] coccinelle: add xxxsetbitsXX converting spatch
From: SF Markus Elfring @ 2018-09-09 11:13 UTC (permalink / raw)
To: Corentin Labbe, kernel-janitors
Cc: Coccinelle, linux-amlogic, linux-arm-kernel, linux-ide,
linux-kernel, linux-sunxi, linuxppc-dev, netdev, Alexandre Torgue,
Alistair Popple, Anatolij Gustschin, Benjamin Herrenschmidt,
Carlo Caione, Chen-Yu Tsai, David S. Miller, Gilles Muller,
Giuseppe Cavallaro, Jose Abreu, Julia Lawall, Kevin Hilman,
Kumar Gala
In-Reply-To: <1536349307-20714-4-git-send-email-clabbe@baylibre.com>
How do you think about to add any more meta-data for this semantic patch script?
* SPDX identifier
* Copyright information
* Confidence level
https://bottest.wiki.kernel.org/coccicheck
> +virtual context
Further variables will be needed if you would like to use corresponding
operation modes (besides “patch”).
> +expression reg;
> +expression set;
> +expression clear;
I propose once more to avoid the repetition of (unnecessary) SmPL code.
This part could be written like the following instead.
+expression clear, set, reg;
If you would increase the usage of SmPL disjunctions, the specification
of duplicate SmPL code could be reduced considerably.
* Would you like to merge SmPL rules based on the distinction between
the data types “u32” and “u64”?
* Did you identify any optional code in this transformation approach?
> +@@
> +expression base;
> +expression offset;
> +expression value;
> +@@
> +
> +- mtu3_setbits(base, offset, value);
> ++ setbits32(base + offset, value);
> +
> +@@
> +expression base;
> +expression offset;
> +expression mask;
> +@@
> +
> +- mtu3_clrbits(base, offset, mask);
> ++ clrbits32(base + offset, mask);
Another update suggestion:
+@replacement@
+expression base, offset;
+@@
+(
+-mtu3_clrbits
++clrbits32
+|
+-mtu3_setbits
++setbits32
+)(base
+- ,
++ +
+ offset, ...);
Would you like to try further software fine-tuning out?
Regards,
Markus
^ permalink raw reply
* Re: [PATCH net] ipv6: use rt6_info members when dst is set in rt6_fill_node
From: Xin Long @ 2018-09-09 6:29 UTC (permalink / raw)
To: David Ahern; +Cc: network dev, davem, Roopa Prabhu
In-Reply-To: <4abf5ec6-862b-f88e-b0ce-ddcd4308fd59@cumulusnetworks.com>
On Sun, Sep 9, 2018 at 9:45 AM David Ahern <dsa@cumulusnetworks.com> wrote:
>
> On 9/8/18 3:24 AM, Xin Long wrote:
> > In inet6_rtm_getroute, since Commit 93531c674315 ("net/ipv6: separate
> > handling of FIB entries from dst based routes"), it has used rt->from
> > to dump route info instead of rt.
> >
> > However for some route like cache, its information is not the same as
> > that of the 'from' one. It caused 'ip -6 route get' to dump the wrong
> > route information.
> >
> > In Jianlin's testing, the output information even lost the expiration
> > time for a pmtu route cache due to the wrong fib6_flags.
>
> you are right about the flags ...
>
> >
> > So change to use rt6_info members when it tries to dump a route entry
> > without fibmatch set.
>
> but not the src, dst and prefsrc.
>
> >
> > Note that we will fix the gw/nh dump in another patch.
>
> And only the gateway can change do to a redirect and redirects do not
> change the device - only the gateway.
>
> Let's do both changes in a single patch.
>
> >
> > Fixes: 93531c674315 ("net/ipv6: separate handling of FIB entries from dst based routes")
> > Reported-by: Jianlin Shi <jishi@redhat.com>
> > Signed-off-by: Xin Long <lucien.xin@gmail.com>
> > ---
> > net/ipv6/route.c | 39 ++++++++++++++++++++++++++-------------
> > 1 file changed, 26 insertions(+), 13 deletions(-)
> >
> > diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> > index 18e00ce..e554922 100644
> > --- a/net/ipv6/route.c
> > +++ b/net/ipv6/route.c
> > @@ -4670,20 +4670,33 @@ static int rt6_fill_node(struct net *net, struct sk_buff *skb,
> > int iif, int type, u32 portid, u32 seq,
> > unsigned int flags)
> > {
> > - struct rtmsg *rtm;
> > + struct rt6key *fib6_prefsrc, *fib6_dst, *fib6_src;
> > + struct rt6_info *rt6 = (struct rt6_info *)dst;
> > + u32 *pmetrics, table, fib6_flags;
> > struct nlmsghdr *nlh;
> > + struct rtmsg *rtm;
> > long expires = 0;
> > - u32 *pmetrics;
> > - u32 table;
> >
> > nlh = nlmsg_put(skb, portid, seq, type, sizeof(*rtm), flags);
> > if (!nlh)
> > return -EMSGSIZE;
> >
> > + if (rt6) {
> > + fib6_dst = &rt6->rt6i_dst;
> > + fib6_src = &rt6->rt6i_src;
> > + fib6_flags = rt6->rt6i_flags;
> > + fib6_prefsrc = &rt6->rt6i_prefsrc;
> > + } else {
> > + fib6_dst = &rt->fib6_dst;
> > + fib6_src = &rt->fib6_src;
> > + fib6_flags = rt->fib6_flags;
> > + fib6_prefsrc = &rt->fib6_prefsrc;
> > + }
>
> Unless I am missing something at the moment, an rt6_info can only have
> the same dst, src and prefsrc as the fib6_info on which it is based.
> Thus, only the flags is needed above. That simplifies this patch a lot.
If dst, src and prefsrc in rt6_info are always the same as these in fib6_info,
why do we need them in rt6_info? we could just get it by 'from'.
^ permalink raw reply
* Re: Overlayfs @ Containers and checkpoint/restart micro-conference at LPC2018
From: Amir Goldstein @ 2018-09-09 6:31 UTC (permalink / raw)
To: christian
Cc: Stephane Graber, containers, Miklos Szeredi, Netdev, overlayfs,
lxc-users, LSM List, lxc-devel, linux-fsdevel, zhangyi (F)
In-Reply-To: <20180909013108.6wjlx75ztqjipepg@mailbox.org>
On Sun, Sep 9, 2018 at 4:31 AM Christian Brauner <christian@brauner.io> wrote:
>
...
> > [cc: overlayfs developers]
> >
> > Hi Stéphane!
>
> Hey Amir,
>
> I'm one of the co-organizers of the microconf.
>
> >
> > I am not planing to travel to LPC this year, so this is more of an FYI than
> > a CFP, but maybe another overlayfs developer can pick up this glove??
>
> Sure, that would be great.
>
> >
> > For the past two years I have participated in the effort to fix overlayfs
> > "non-standard" behavior:
> > https://github.com/amir73il/overlayfs/wiki/Overlayfs-non-standard-behavior
>
> Yes, this is an issue that we were aware of for a long time and it
> something that has made overlayfs somewhat more difficult to use than it
> should be.
>
> >
> > Allegedly, this effort went underway to improve the experience of overlayfs
> > users, who are mostly applications running inside containers. For backward
> > compatibility reasons, container runtimes will need to opt-in for fixing some
> > of the legacy behavior.
> >
> > In reality, I have seen very little cross list interaction between linux-unionfs
> > and containers mailing lists. The only interaction I recall in the
> > past two years
> > ended up in a fix in overlayfs to require opt-in for fixing yet another backward
> > compatible bad behavior, although docker did follow up shortly after to fix
> > bad practice in container runtime:
> > https://github.com/moby/moby/issues/34672
> >
> > So the questions I would like to relay to the micro-conf participants w.r.t the
> > new opt-in overlayfs behavior:
> > 1. Did you know?
>
> I personally did not know about the new opt-in behavior. More reason to
> give a talk! :)
>
> > 2. Do you care?
>
> Yes, we do care. However - speaking as LXC upstream now - we have
> recently focused on getting shiftfs to work rather than overlayfs.
>
IMO, as I expressed it in the past, the fact that shiftfs development is not
collaborated with overlayfs developers is a pitty.
Yes shiftfs has a different purpose than overlayfs, but they have common
use cases and common problems as well.
> We are more than happy to have a overlayfs talk at the microconf. If
> someone were to talk about:
> - What non-standard behavior has already been fixed?
> - How has it been fixed?
IMO, those questions are covered quite well by the wiki and overlayfs.txt
documentation in kernel tree.
> - What non-standard behavior still needs to be fixed?
There's the mmap MAP_SHARED case covered in the wiki
and there may be other small stuff, but not sure if anyone cares
about them, so the question should really be directed back to the audience...
> - Outstanding problems that either still need a solution or
> are solved but one would like feedback on the implementation. This way
> we can have a good discussion.
>
I think one of the chsallange that distros and container runtime will need to
deal with is managing format versions of overlay "images".
The reason the new features require user or distro to opt-in is because
the new features create overlayfs images that are not fully compatible with old
kernels and existing container image tools (i.e. export/migrate image).
The new overlayfs-progs project by Zhangyi is going to help in that respect:
https://github.com/hisilicon/overlayfs-progs
As well as Zhangyi's work on overlayfs feature set support:
https://marc.info/?l=linux-unionfs&m=153302911328159&w=2
Thanks,
Amir.
^ permalink raw reply
* Re: [PATCH net 01/13] net: sched: cls_u32: fix hnode refcounting
From: Jamal Hadi Salim @ 2018-09-09 11:37 UTC (permalink / raw)
To: Al Viro, netdev; +Cc: Cong Wang, Jiri Pirko, stable
In-Reply-To: <20180909013132.3222-2-viro@ZenIV.linux.org.uk>
On 2018-09-08 9:31 p.m., Al Viro wrote:
> From: Al Viro <viro@zeniv.linux.org.uk>
>
> cls_u32.c misuses refcounts for struct tc_u_hnode - it counts references via
> ->hlist and via ->tp_root together. u32_destroy() drops the former and, in
> case when there had been links, leaves the sucker on the list. As the result,
> there's nothing to protect it from getting freed once links are dropped.
> That also makes the "is it busy" check incapable of catching the root hnode -
> it *is* busy (there's a reference from tp), but we don't see it as something
> separate. "Is it our root?" check partially covers that, but the problem
> exists for others' roots as well.
>
> AFAICS, the minimal fix preserving the existing behaviour (where it doesn't
> include oopsen, that is) would be this:
> * count tp->root and tp_c->hlist as separate references. I.e.
> have u32_init() set refcount to 2, not 1.
> * in u32_destroy() we always drop the former; in u32_destroy_hnode() -
> the latter.
>
> That way we have *all* references contributing to refcount. List
> removal happens in u32_destroy_hnode() (called only when ->refcnt is 1)
> an in u32_destroy() in case of tc_u_common going away, along with everything
> reachable from it. IOW, that way we know that u32_destroy_key() won't
> free something still on the list (or pointed to by someone's ->root).
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
per Cong's earlier remark - the reproducer going in the changelog
would be nice to show i.e this you posted earlier, otherwise:
Tested-by: Jamal Hadi Salim <jhs@mojatatu.com>
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
Reminder:
^ 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