| Message ID | 20220216020009.3404578-1-jmaloy@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | c08e58438d4a709fb451b6d7d33432cc9907a2a8 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v2,net] tipc: fix wrong notification node addresses | expand |
On Tue, Feb 15, 2022 at 09:00:09PM -0500, jmaloy@redhat.com wrote: > From: Jon Maloy <jmaloy@redhat.com> > > The previous bug fix had an unfortunate side effect that broke > distribution of binding table entries between nodes. The updated > tipc_sock_addr struct is also used further down in the same > function, and there the old value is still the correct one. > > We fix this now. > > Fixes: 032062f363b4 ("tipc: fix wrong publisher node address in link publications") > Please don't put blank lines between Fixes and SOB lines. Thanks > Signed-off-by: Jon Maloy <jmaloy@redhat.com> > > --- > v2: Copied n->addr to stack variable before leaving lock context, and > using this in the notifications. > --- > net/tipc/node.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/net/tipc/node.c b/net/tipc/node.c > index fd95df338da7..6ef95ce565bd 100644 > --- a/net/tipc/node.c > +++ b/net/tipc/node.c > @@ -403,7 +403,7 @@ static void tipc_node_write_unlock(struct tipc_node *n) > u32 flags = n->action_flags; > struct list_head *publ_list; > struct tipc_uaddr ua; > - u32 bearer_id; > + u32 bearer_id, node; > > if (likely(!flags)) { > write_unlock_bh(&n->lock); > @@ -414,6 +414,7 @@ static void tipc_node_write_unlock(struct tipc_node *n) > TIPC_LINK_STATE, n->addr, n->addr); > sk.ref = n->link_id; > sk.node = tipc_own_addr(net); > + node = n->addr; > bearer_id = n->link_id & 0xffff; > publ_list = &n->publ_list; > > @@ -423,17 +424,17 @@ static void tipc_node_write_unlock(struct tipc_node *n) > write_unlock_bh(&n->lock); > > if (flags & TIPC_NOTIFY_NODE_DOWN) > - tipc_publ_notify(net, publ_list, sk.node, n->capabilities); > + tipc_publ_notify(net, publ_list, node, n->capabilities); > > if (flags & TIPC_NOTIFY_NODE_UP) > - tipc_named_node_up(net, sk.node, n->capabilities); > + tipc_named_node_up(net, node, n->capabilities); > > if (flags & TIPC_NOTIFY_LINK_UP) { > - tipc_mon_peer_up(net, sk.node, bearer_id); > + tipc_mon_peer_up(net, node, bearer_id); > tipc_nametbl_publish(net, &ua, &sk, sk.ref); > } > if (flags & TIPC_NOTIFY_LINK_DOWN) { > - tipc_mon_peer_down(net, sk.node, bearer_id); > + tipc_mon_peer_down(net, node, bearer_id); > tipc_nametbl_withdraw(net, &ua, &sk, sk.ref); > } > } > -- > 2.31.1 >
On 2/16/22 02:08, Leon Romanovsky wrote: > On Tue, Feb 15, 2022 at 09:00:09PM -0500, jmaloy@redhat.com wrote: >> From: Jon Maloy <jmaloy@redhat.com> >> >> The previous bug fix had an unfortunate side effect that broke >> distribution of binding table entries between nodes. The updated >> tipc_sock_addr struct is also used further down in the same >> function, and there the old value is still the correct one. >> >> We fix this now. >> >> Fixes: 032062f363b4 ("tipc: fix wrong publisher node address in link publications") >> > Please don't put blank lines between Fixes and SOB lines. > > Thanks Seems like somebody should update the checkpatch.pl script. ///jon > >> Signed-off-by: Jon Maloy <jmaloy@redhat.com> >> >> --- >> v2: Copied n->addr to stack variable before leaving lock context, and >> using this in the notifications. >> --- >> net/tipc/node.c | 11 ++++++----- >> 1 file changed, 6 insertions(+), 5 deletions(-) >> >> diff --git a/net/tipc/node.c b/net/tipc/node.c >> index fd95df338da7..6ef95ce565bd 100644 >> --- a/net/tipc/node.c >> +++ b/net/tipc/node.c >> @@ -403,7 +403,7 @@ static void tipc_node_write_unlock(struct tipc_node *n) >> u32 flags = n->action_flags; >> struct list_head *publ_list; >> struct tipc_uaddr ua; >> - u32 bearer_id; >> + u32 bearer_id, node; >> >> if (likely(!flags)) { >> write_unlock_bh(&n->lock); >> @@ -414,6 +414,7 @@ static void tipc_node_write_unlock(struct tipc_node *n) >> TIPC_LINK_STATE, n->addr, n->addr); >> sk.ref = n->link_id; >> sk.node = tipc_own_addr(net); >> + node = n->addr; >> bearer_id = n->link_id & 0xffff; >> publ_list = &n->publ_list; >> >> @@ -423,17 +424,17 @@ static void tipc_node_write_unlock(struct tipc_node *n) >> write_unlock_bh(&n->lock); >> >> if (flags & TIPC_NOTIFY_NODE_DOWN) >> - tipc_publ_notify(net, publ_list, sk.node, n->capabilities); >> + tipc_publ_notify(net, publ_list, node, n->capabilities); >> >> if (flags & TIPC_NOTIFY_NODE_UP) >> - tipc_named_node_up(net, sk.node, n->capabilities); >> + tipc_named_node_up(net, node, n->capabilities); >> >> if (flags & TIPC_NOTIFY_LINK_UP) { >> - tipc_mon_peer_up(net, sk.node, bearer_id); >> + tipc_mon_peer_up(net, node, bearer_id); >> tipc_nametbl_publish(net, &ua, &sk, sk.ref); >> } >> if (flags & TIPC_NOTIFY_LINK_DOWN) { >> - tipc_mon_peer_down(net, sk.node, bearer_id); >> + tipc_mon_peer_down(net, node, bearer_id); >> tipc_nametbl_withdraw(net, &ua, &sk, sk.ref); >> } >> } >> -- >> 2.31.1 >>
On Wed, Feb 16, 2022 at 10:12:44AM -0500, Jon Maloy wrote: > > > On 2/16/22 02:08, Leon Romanovsky wrote: > > On Tue, Feb 15, 2022 at 09:00:09PM -0500, jmaloy@redhat.com wrote: > > > From: Jon Maloy <jmaloy@redhat.com> > > > > > > The previous bug fix had an unfortunate side effect that broke > > > distribution of binding table entries between nodes. The updated > > > tipc_sock_addr struct is also used further down in the same > > > function, and there the old value is still the correct one. > > > > > > We fix this now. > > > > > > Fixes: 032062f363b4 ("tipc: fix wrong publisher node address in link publications") > > > > > Please don't put blank lines between Fixes and SOB lines. > > > > Thanks > Seems like somebody should update the checkpatch.pl script. Patches are welcomed :) > > ///jon > > > > > > Signed-off-by: Jon Maloy <jmaloy@redhat.com> > > > > > > --- > > > v2: Copied n->addr to stack variable before leaving lock context, and > > > using this in the notifications. > > > --- > > > net/tipc/node.c | 11 ++++++----- > > > 1 file changed, 6 insertions(+), 5 deletions(-) > > > > > > diff --git a/net/tipc/node.c b/net/tipc/node.c > > > index fd95df338da7..6ef95ce565bd 100644 > > > --- a/net/tipc/node.c > > > +++ b/net/tipc/node.c > > > @@ -403,7 +403,7 @@ static void tipc_node_write_unlock(struct tipc_node *n) > > > u32 flags = n->action_flags; > > > struct list_head *publ_list; > > > struct tipc_uaddr ua; > > > - u32 bearer_id; > > > + u32 bearer_id, node; > > > if (likely(!flags)) { > > > write_unlock_bh(&n->lock); > > > @@ -414,6 +414,7 @@ static void tipc_node_write_unlock(struct tipc_node *n) > > > TIPC_LINK_STATE, n->addr, n->addr); > > > sk.ref = n->link_id; > > > sk.node = tipc_own_addr(net); > > > + node = n->addr; > > > bearer_id = n->link_id & 0xffff; > > > publ_list = &n->publ_list; > > > @@ -423,17 +424,17 @@ static void tipc_node_write_unlock(struct tipc_node *n) > > > write_unlock_bh(&n->lock); > > > if (flags & TIPC_NOTIFY_NODE_DOWN) > > > - tipc_publ_notify(net, publ_list, sk.node, n->capabilities); > > > + tipc_publ_notify(net, publ_list, node, n->capabilities); > > > if (flags & TIPC_NOTIFY_NODE_UP) > > > - tipc_named_node_up(net, sk.node, n->capabilities); > > > + tipc_named_node_up(net, node, n->capabilities); > > > if (flags & TIPC_NOTIFY_LINK_UP) { > > > - tipc_mon_peer_up(net, sk.node, bearer_id); > > > + tipc_mon_peer_up(net, node, bearer_id); > > > tipc_nametbl_publish(net, &ua, &sk, sk.ref); > > > } > > > if (flags & TIPC_NOTIFY_LINK_DOWN) { > > > - tipc_mon_peer_down(net, sk.node, bearer_id); > > > + tipc_mon_peer_down(net, node, bearer_id); > > > tipc_nametbl_withdraw(net, &ua, &sk, sk.ref); > > > } > > > } > > > -- > > > 2.31.1 > > > >
Hello: This patch was applied to netdev/net.git (master) by Jakub Kicinski <kuba@kernel.org>: On Tue, 15 Feb 2022 21:00:09 -0500 you wrote: > From: Jon Maloy <jmaloy@redhat.com> > > The previous bug fix had an unfortunate side effect that broke > distribution of binding table entries between nodes. The updated > tipc_sock_addr struct is also used further down in the same > function, and there the old value is still the correct one. > > [...] Here is the summary with links: - [v2,net] tipc: fix wrong notification node addresses https://git.kernel.org/netdev/net/c/c08e58438d4a You are awesome, thank you!
diff --git a/net/tipc/node.c b/net/tipc/node.c index fd95df338da7..6ef95ce565bd 100644 --- a/net/tipc/node.c +++ b/net/tipc/node.c @@ -403,7 +403,7 @@ static void tipc_node_write_unlock(struct tipc_node *n) u32 flags = n->action_flags; struct list_head *publ_list; struct tipc_uaddr ua; - u32 bearer_id; + u32 bearer_id, node; if (likely(!flags)) { write_unlock_bh(&n->lock); @@ -414,6 +414,7 @@ static void tipc_node_write_unlock(struct tipc_node *n) TIPC_LINK_STATE, n->addr, n->addr); sk.ref = n->link_id; sk.node = tipc_own_addr(net); + node = n->addr; bearer_id = n->link_id & 0xffff; publ_list = &n->publ_list; @@ -423,17 +424,17 @@ static void tipc_node_write_unlock(struct tipc_node *n) write_unlock_bh(&n->lock); if (flags & TIPC_NOTIFY_NODE_DOWN) - tipc_publ_notify(net, publ_list, sk.node, n->capabilities); + tipc_publ_notify(net, publ_list, node, n->capabilities); if (flags & TIPC_NOTIFY_NODE_UP) - tipc_named_node_up(net, sk.node, n->capabilities); + tipc_named_node_up(net, node, n->capabilities); if (flags & TIPC_NOTIFY_LINK_UP) { - tipc_mon_peer_up(net, sk.node, bearer_id); + tipc_mon_peer_up(net, node, bearer_id); tipc_nametbl_publish(net, &ua, &sk, sk.ref); } if (flags & TIPC_NOTIFY_LINK_DOWN) { - tipc_mon_peer_down(net, sk.node, bearer_id); + tipc_mon_peer_down(net, node, bearer_id); tipc_nametbl_withdraw(net, &ua, &sk, sk.ref); } }