[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <64ba0629-6481-41d2-ad99-38d296e93206@kernel.org>
Date: Wed, 3 Apr 2024 11:49:24 -0600
From: David Ahern <dsahern@...nel.org>
To: Eric Dumazet <edumazet@...gle.com>
Cc: "David S . Miller" <davem@...emloft.net>, Jakub Kicinski
<kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org,
eric.dumazet@...il.com
Subject: Re: [PATCH net-next] ipv6: remove RTNL protection from
ip6addrlbl_dump()
On 4/3/24 10:25 AM, Eric Dumazet wrote:
> On Wed, Apr 3, 2024 at 6:13 PM David Ahern <dsahern@...nel.org> wrote:
>>
>> On 4/3/24 6:39 AM, Eric Dumazet wrote:
>>> diff --git a/net/ipv6/addrlabel.c b/net/ipv6/addrlabel.c
>>> index 17ac45aa7194ce9c148ed95e14dd575d17feeb98..961e853543b64cd2060ef693ae3ad32a44780aa1 100644
>>> --- a/net/ipv6/addrlabel.c
>>> +++ b/net/ipv6/addrlabel.c
>>> @@ -234,7 +234,8 @@ static int __ip6addrlbl_add(struct net *net, struct ip6addrlbl_entry *newp,
>>> hlist_add_head_rcu(&newp->list, &net->ipv6.ip6addrlbl_table.head);
>>> out:
>>> if (!ret)
>>> - net->ipv6.ip6addrlbl_table.seq++;
>>> + WRITE_ONCE(net->ipv6.ip6addrlbl_table.seq,
>>> + net->ipv6.ip6addrlbl_table.seq + 1);
>>> return ret;
>>> }
>>>
>>> @@ -445,7 +446,7 @@ static void ip6addrlbl_putmsg(struct nlmsghdr *nlh,
>>> };
>>>
>>> static int ip6addrlbl_fill(struct sk_buff *skb,
>>> - struct ip6addrlbl_entry *p,
>>> + const struct ip6addrlbl_entry *p,
>>> u32 lseq,
>>> u32 portid, u32 seq, int event,
>>> unsigned int flags)
>>> @@ -498,7 +499,7 @@ static int ip6addrlbl_dump(struct sk_buff *skb, struct netlink_callback *cb)
>>> struct net *net = sock_net(skb->sk);
>>> struct ip6addrlbl_entry *p;
>>> int idx = 0, s_idx = cb->args[0];
>>> - int err;
>>> + int err = 0;
>>>
>>> if (cb->strict_check) {
>>> err = ip6addrlbl_valid_dump_req(nlh, cb->extack);
>>> @@ -510,7 +511,7 @@ static int ip6addrlbl_dump(struct sk_buff *skb, struct netlink_callback *cb)
>>> hlist_for_each_entry_rcu(p, &net->ipv6.ip6addrlbl_table.head, list) {
>>> if (idx >= s_idx) {
>>> err = ip6addrlbl_fill(skb, p,
>>> - net->ipv6.ip6addrlbl_table.seq,
>>> + READ_ONCE(net->ipv6.ip6addrlbl_table.seq),
>>
>> seems like this should be read once on entry, and the same value used
>> for all iterations.
>
> I thought of that, but this will miss any update done concurrently (if
> all entries fit in a single skb)
>
> It is unclear to me what user space can do with ifal_seq
>
> There is no clear signal like :
>
> Initial seq _before_ the dump, and seq seen _after_ the dump.
Since this a known change in behavior (however slight), it would be best
to add something in the commit message or a code comment so if someone
hits a problem and does a git bisect the problem and solution are clear.
Powered by blists - more mailing lists