tipc-discussion — User questions & problem reports, project news, developer discussion

Re: [tipc-discussion] [PATCH net 1/2] tipc: add tipc_bearer_min_mtu to calculate min mtu

[Xin L. <luc...@gm...>]

On Mon, May 1, 2023 at 11:35 AM Xin Long <luc...@gm...> wrote:
> On Mon, May 1, 2023 at 1:21 AM Tung Quang Nguyen <tun...@de...> wrote:
>> >@@ -760,6 +760,7 @@ static int tipc_udp_enable(struct net *net, struct tipc_bearer *b,
>> >               else
>> >                       udp_conf.local_ip6 = local.ipv6;
>> >               ub->ifindex = dev->ifindex;
>> >+              b->encap_hlen = sizeof(struct ipv6hdr) + sizeof(struct udphdr);
>> tipc_mtu_bad() needs to be called here to check for the minimum required MTU like the way ipv4 UDP bearer does.
>
> Agree, especially after commit 5a6f6f579178 ("tipc: set ub->ifindex for local ipv6 address"), we have the dev there.
After taking a second look, I think we should delete the tipc_mtu_bad()
call for ipv4 UDP bearer, as b->mtu is no longer using dev->mtu since:

  commit a4dfa72d0acd ("tipc: set default MTU for UDP media")

The issue described in commit 3de81b758853 ("tipc: check minimum bearer MTU")
no longer exists in UDP bearer.

Besides, dev->mtu can still be changed to a too small mtu after the UDP
bearer is created even with the tipc_mtu_bad() check in tipc_udp_enable().
Note that NETDEV_CHANGEMTU event processing in tipc_l2_device_event()
doesn't really work for UDP bearer.

I will leave this patch as it is in my v2 post, and create another patch
for net-next to delete the unnecessary tipc_mtu_bad() check in UDP bearer.

Agree?

Thanks.

Re: [tipc-discussion] [PATCH net 2/2] tipc: do not update mtu if msg_max is too small

[Xin L. <luc...@gm...>]

On Mon, May 1, 2023 at 1:22 AM Tung Quang Nguyen <
tun...@de...> wrote:

>
>
> >-----Original Message-----
> >From: Xin Long <luc...@gm...>
> >Sent: Sunday, April 30, 2023 5:41 AM
> >To: network dev <ne...@vg...>;
> tip...@li...
> >Cc: da...@da...; ku...@ke...; Eric Dumazet <
> edu...@go...>; Paolo Abeni <pa...@re...>; Jon Maloy
> ><jm...@re...>
> >Subject: [PATCH net 2/2] tipc: do not update mtu if msg_max is too small
> >
> >When doing link mtu negotiation, a malicious peer may send Activate msg
> >with a very small mtu, e.g. 4 in Shuang's testing, without checking for
> >the minimum mtu, l->mtu will be set to 4 in tipc_link_proto_rcv(), then
> >n->links[bearer_id].mtu is set to 4294967228, which is a overflow of
> >'4 - INT_H_SIZE - EMSG_OVERHEAD' in tipc_link_mss().
> >
> >With tipc_link.mtu = 4, tipc_link_xmit() kept printing the warning:
> >
> > tipc: Too large msg, purging xmit list 1 5 0 40 4!
> > tipc: Too large msg, purging xmit list 1 15 0 60 4!
> >
> >And with tipc_link_entry.mtu 4294967228, a huge skb was allocated in
> >named_distribute(), and when purging it in tipc_link_xmit(), a crash
> >was even caused:
> >
> >  general protection fault, probably for non-canonical address
> 0x2100001011000dd: 0000 [#1] PREEMPT SMP PTI
> >  CPU: 0 PID: 0 Comm: swapper/0 Kdump: loaded Not tainted 6.3.0.neta #19
> >  RIP: 0010:kfree_skb_list_reason+0x7e/0x1f0
> >  Call Trace:
> >   <IRQ>
> >   skb_release_data+0xf9/0x1d0
> >   kfree_skb_reason+0x40/0x100
> >   tipc_link_xmit+0x57a/0x740 [tipc]
> >   tipc_node_xmit+0x16c/0x5c0 [tipc]
> >   tipc_named_node_up+0x27f/0x2c0 [tipc]
> >   tipc_node_write_unlock+0x149/0x170 [tipc]
> >   tipc_rcv+0x608/0x740 [tipc]
> >   tipc_udp_recv+0xdc/0x1f0 [tipc]
> >   udp_queue_rcv_one_skb+0x33e/0x620
> >   udp_unicast_rcv_skb.isra.72+0x75/0x90
> >   __udp4_lib_rcv+0x56d/0xc20
> >   ip_protocol_deliver_rcu+0x100/0x2d0
> >
> >This patch fixes it by checking the new mtu against tipc_bearer_min_mtu(),
> >and not updating mtu if it is too small.
> >
> >Fixes: ed193ece2649 ("tipc: simplify link mtu negotiation")
> >Reported-by: Shuang Li <sh...@re...>
> >Signed-off-by: Xin Long <luc...@gm...>
> >---
> > net/tipc/link.c | 7 ++++---
> > 1 file changed, 4 insertions(+), 3 deletions(-)
> >
> >diff --git a/net/tipc/link.c b/net/tipc/link.c
> >index b3ce24823f50..a9e46c58b28a 100644
> >--- a/net/tipc/link.c
> >+++ b/net/tipc/link.c
> >@@ -2200,7 +2200,7 @@ static int tipc_link_proto_rcv(struct tipc_link *l,
> struct sk_buff *skb,
> >       struct tipc_msg *hdr = buf_msg(skb);
> >       struct tipc_gap_ack_blks *ga = NULL;
> >       bool reply = msg_probe(hdr), retransmitted = false;
> >-      u32 dlen = msg_data_sz(hdr), glen = 0;
> >+      u32 dlen = msg_data_sz(hdr), glen = 0, msg_max;
> >       u16 peers_snd_nxt =  msg_next_sent(hdr);
> >       u16 peers_tol = msg_link_tolerance(hdr);
> >       u16 peers_prio = msg_linkprio(hdr);
> >@@ -2283,8 +2283,9 @@ static int tipc_link_proto_rcv(struct tipc_link *l,
> struct sk_buff *skb,
> >               l->peer_session = msg_session(hdr);
> >               l->in_session = true;
> >               l->peer_bearer_id = msg_bearer_id(hdr);
> >-              if (l->mtu > msg_max_pkt(hdr))
> >-                      l->mtu = msg_max_pkt(hdr);
> >+              msg_max = msg_max_pkt(hdr);
> >+              if (msg_max >= tipc_bearer_min_mtu(l->net, l->bearer_id)
> && l->mtu > msg_max)
> >+                      l->mtu = msg_max;
> If this link receives a malicious ACTIVATE_MSG from a peer, this message
> should be dropped. It is better if the check " msg_max <
> tipc_bearer_min_mtu()" is put at the beginning of this ACTIVATE_MSG
> handling and we break immediately.
>
> Sounds good, will move 'msg_max < tipc_bearer_min_mtu()' up.

Thanks for reviewing.

Re: [tipc-discussion] [PATCH net 1/2] tipc: add tipc_bearer_min_mtu to calculate min mtu

[Xin L. <luc...@gm...>]

On Mon, May 1, 2023 at 1:21 AM Tung Quang Nguyen <
tun...@de...> wrote:

>
>
> >-----Original Message-----
> >From: Xin Long <luc...@gm...>
> >Sent: Sunday, April 30, 2023 5:41 AM
> >To: network dev <ne...@vg...>;
> tip...@li...
> >Cc: ku...@ke...; Eric Dumazet <edu...@go...>; Paolo Abeni <
> pa...@re...>; da...@da...
> >Subject: [tipc-discussion] [PATCH net 1/2] tipc: add tipc_bearer_min_mtu
> to calculate min mtu
> >
> >As different media may requires different min mtu, and even the
> >same media with different net family requires different min mtu,
> >add tipc_bearer_min_mtu() to calculate min mtu accordingly.
> >
> >This API will be used to check the new mtu when doing the link
> >mtu negotiation in the next patch.
> >
> >Signed-off-by: Xin Long <luc...@gm...>
> >---
> > net/tipc/bearer.c    | 13 +++++++++++++
> > net/tipc/bearer.h    |  3 +++
> > net/tipc/udp_media.c |  5 +++--
> > 3 files changed, 19 insertions(+), 2 deletions(-)
> >
> >diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c
> >index 35cac7733fd3..c5d2e8c45f88 100644
> >--- a/net/tipc/bearer.c
> >+++ b/net/tipc/bearer.c
> >@@ -541,6 +541,19 @@ int tipc_bearer_mtu(struct net *net, u32 bearer_id)
> >       return mtu;
> > }
> >
> >+int tipc_bearer_min_mtu(struct net *net, u32 bearer_id)
> >+{
> >+      int mtu = TIPC_MIN_BEARER_MTU;
> >+      struct tipc_bearer *b;
> >+
> >+      rcu_read_lock();
> >+      b = rcu_dereference(tipc_net(net)->bearer_list[bearer_id]);
> >+      if (b)
> >+              mtu += b->encap_hlen;
> >+      rcu_read_unlock();
> >+      return mtu;
> >+}
> >+
> > /* tipc_bearer_xmit_skb - sends buffer to destination over bearer
> >  */
> > void tipc_bearer_xmit_skb(struct net *net, u32 bearer_id,
> >diff --git a/net/tipc/bearer.h b/net/tipc/bearer.h
> >index 490ad6e5f7a3..bd0cc5c287ef 100644
> >--- a/net/tipc/bearer.h
> >+++ b/net/tipc/bearer.h
> >@@ -146,6 +146,7 @@ struct tipc_media {
> >  * @identity: array index of this bearer within TIPC bearer array
> >  * @disc: ptr to link setup request
> >  * @net_plane: network plane ('A' through 'H') currently associated with
> bearer
> >+ * @encap_hlen: encap headers length
> >  * @up: bearer up flag (bit 0)
> >  * @refcnt: tipc_bearer reference counter
> >  *
> >@@ -170,6 +171,7 @@ struct tipc_bearer {
> >       u32 identity;
> >       struct tipc_discoverer *disc;
> >       char net_plane;
> >+      u16 encap_hlen;
> >       unsigned long up;
> >       refcount_t refcnt;
> > };
> >@@ -232,6 +234,7 @@ int tipc_bearer_setup(void);
> > void tipc_bearer_cleanup(void);
> > void tipc_bearer_stop(struct net *net);
> > int tipc_bearer_mtu(struct net *net, u32 bearer_id);
> >+int tipc_bearer_min_mtu(struct net *net, u32 bearer_id);
> > bool tipc_bearer_bcast_support(struct net *net, u32 bearer_id);
> > void tipc_bearer_xmit_skb(struct net *net, u32 bearer_id,
> >                         struct sk_buff *skb,
> >diff --git a/net/tipc/udp_media.c b/net/tipc/udp_media.c
> >index c2bb818704c8..0a85244fd618 100644
> >--- a/net/tipc/udp_media.c
> >+++ b/net/tipc/udp_media.c
> >@@ -738,8 +738,8 @@ static int tipc_udp_enable(struct net *net, struct
> tipc_bearer *b,
> >                       udp_conf.local_ip.s_addr = local.ipv4.s_addr;
> >               udp_conf.use_udp_checksums = false;
> >               ub->ifindex = dev->ifindex;
> >-              if (tipc_mtu_bad(dev, sizeof(struct iphdr) +
> >-                                    sizeof(struct udphdr))) {
> >+              b->encap_hlen = sizeof(struct iphdr) + sizeof(struct
> udphdr);
> >+              if (tipc_mtu_bad(dev, b->encap_hlen)) {
> >                       err = -EINVAL;
> >                       goto err;
> >               }
> >@@ -760,6 +760,7 @@ static int tipc_udp_enable(struct net *net, struct
> tipc_bearer *b,
> >               else
> >                       udp_conf.local_ip6 = local.ipv6;
> >               ub->ifindex = dev->ifindex;
> >+              b->encap_hlen = sizeof(struct ipv6hdr) + sizeof(struct
> udphdr);
> tipc_mtu_bad() needs to be called here to check for the minimum required
> MTU like the way ipv4 UDP bearer does.

Agree, especially after commit 5a6f6f579178 ("tipc: set ub->ifindex for
local ipv6 address"), we have the dev there.

Thanks.

>               b->mtu = 1280;
> > #endif
> >       } else {
> >--
> >2.39.1
> >
> >
> >
> >_______________________________________________
> >tipc-discussion mailing list
> >tip...@li...
> >https://lists.sourceforge.net/lists/listinfo/tipc-discussion
>

Re: [tipc-discussion] [PATCH net 2/2] tipc: do not update mtu if msg_max is too small

[Tung Q. N. <tun...@de...>]

>-----Original Message-----
>From: Xin Long <luc...@gm...>
>Sent: Sunday, April 30, 2023 5:41 AM
>To: network dev <ne...@vg...>; tip...@li...
>Cc: da...@da...; ku...@ke...; Eric Dumazet <edu...@go...>; Paolo Abeni <pa...@re...>; Jon Maloy
><jm...@re...>
>Subject: [PATCH net 2/2] tipc: do not update mtu if msg_max is too small
>
>When doing link mtu negotiation, a malicious peer may send Activate msg
>with a very small mtu, e.g. 4 in Shuang's testing, without checking for
>the minimum mtu, l->mtu will be set to 4 in tipc_link_proto_rcv(), then
>n->links[bearer_id].mtu is set to 4294967228, which is a overflow of
>'4 - INT_H_SIZE - EMSG_OVERHEAD' in tipc_link_mss().
>
>With tipc_link.mtu = 4, tipc_link_xmit() kept printing the warning:
>
> tipc: Too large msg, purging xmit list 1 5 0 40 4!
> tipc: Too large msg, purging xmit list 1 15 0 60 4!
>
>And with tipc_link_entry.mtu 4294967228, a huge skb was allocated in
>named_distribute(), and when purging it in tipc_link_xmit(), a crash
>was even caused:
>
>  general protection fault, probably for non-canonical address 0x2100001011000dd: 0000 [#1] PREEMPT SMP PTI
>  CPU: 0 PID: 0 Comm: swapper/0 Kdump: loaded Not tainted 6.3.0.neta #19
>  RIP: 0010:kfree_skb_list_reason+0x7e/0x1f0
>  Call Trace:
>   <IRQ>
>   skb_release_data+0xf9/0x1d0
>   kfree_skb_reason+0x40/0x100
>   tipc_link_xmit+0x57a/0x740 [tipc]
>   tipc_node_xmit+0x16c/0x5c0 [tipc]
>   tipc_named_node_up+0x27f/0x2c0 [tipc]
>   tipc_node_write_unlock+0x149/0x170 [tipc]
>   tipc_rcv+0x608/0x740 [tipc]
>   tipc_udp_recv+0xdc/0x1f0 [tipc]
>   udp_queue_rcv_one_skb+0x33e/0x620
>   udp_unicast_rcv_skb.isra.72+0x75/0x90
>   __udp4_lib_rcv+0x56d/0xc20
>   ip_protocol_deliver_rcu+0x100/0x2d0
>
>This patch fixes it by checking the new mtu against tipc_bearer_min_mtu(),
>and not updating mtu if it is too small.
>
>Fixes: ed193ece2649 ("tipc: simplify link mtu negotiation")
>Reported-by: Shuang Li <sh...@re...>
>Signed-off-by: Xin Long <luc...@gm...>
>---
> net/tipc/link.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
>diff --git a/net/tipc/link.c b/net/tipc/link.c
>index b3ce24823f50..a9e46c58b28a 100644
>--- a/net/tipc/link.c
>+++ b/net/tipc/link.c
>@@ -2200,7 +2200,7 @@ static int tipc_link_proto_rcv(struct tipc_link *l, struct sk_buff *skb,
> 	struct tipc_msg *hdr = buf_msg(skb);
> 	struct tipc_gap_ack_blks *ga = NULL;
> 	bool reply = msg_probe(hdr), retransmitted = false;
>-	u32 dlen = msg_data_sz(hdr), glen = 0;
>+	u32 dlen = msg_data_sz(hdr), glen = 0, msg_max;
> 	u16 peers_snd_nxt =  msg_next_sent(hdr);
> 	u16 peers_tol = msg_link_tolerance(hdr);
> 	u16 peers_prio = msg_linkprio(hdr);
>@@ -2283,8 +2283,9 @@ static int tipc_link_proto_rcv(struct tipc_link *l, struct sk_buff *skb,
> 		l->peer_session = msg_session(hdr);
> 		l->in_session = true;
> 		l->peer_bearer_id = msg_bearer_id(hdr);
>-		if (l->mtu > msg_max_pkt(hdr))
>-			l->mtu = msg_max_pkt(hdr);
>+		msg_max = msg_max_pkt(hdr);
>+		if (msg_max >= tipc_bearer_min_mtu(l->net, l->bearer_id) && l->mtu > msg_max)
>+			l->mtu = msg_max;
If this link receives a malicious ACTIVATE_MSG from a peer, this message should be dropped. It is better if the check " msg_max < tipc_bearer_min_mtu()" is put at the beginning of this ACTIVATE_MSG handling and we break immediately.
> 		break;
>
> 	case STATE_MSG:
>--
>2.39.1

Re: [tipc-discussion] [PATCH net 1/2] tipc: add tipc_bearer_min_mtu to calculate min mtu

[Tung Q. N. <tun...@de...>]

>-----Original Message-----
>From: Xin Long <luc...@gm...>
>Sent: Sunday, April 30, 2023 5:41 AM
>To: network dev <ne...@vg...>; tip...@li...
>Cc: ku...@ke...; Eric Dumazet <edu...@go...>; Paolo Abeni <pa...@re...>; da...@da...
>Subject: [tipc-discussion] [PATCH net 1/2] tipc: add tipc_bearer_min_mtu to calculate min mtu
>
>As different media may requires different min mtu, and even the
>same media with different net family requires different min mtu,
>add tipc_bearer_min_mtu() to calculate min mtu accordingly.
>
>This API will be used to check the new mtu when doing the link
>mtu negotiation in the next patch.
>
>Signed-off-by: Xin Long <luc...@gm...>
>---
> net/tipc/bearer.c    | 13 +++++++++++++
> net/tipc/bearer.h    |  3 +++
> net/tipc/udp_media.c |  5 +++--
> 3 files changed, 19 insertions(+), 2 deletions(-)
>
>diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c
>index 35cac7733fd3..c5d2e8c45f88 100644
>--- a/net/tipc/bearer.c
>+++ b/net/tipc/bearer.c
>@@ -541,6 +541,19 @@ int tipc_bearer_mtu(struct net *net, u32 bearer_id)
> 	return mtu;
> }
>
>+int tipc_bearer_min_mtu(struct net *net, u32 bearer_id)
>+{
>+	int mtu = TIPC_MIN_BEARER_MTU;
>+	struct tipc_bearer *b;
>+
>+	rcu_read_lock();
>+	b = rcu_dereference(tipc_net(net)->bearer_list[bearer_id]);
>+	if (b)
>+		mtu += b->encap_hlen;
>+	rcu_read_unlock();
>+	return mtu;
>+}
>+
> /* tipc_bearer_xmit_skb - sends buffer to destination over bearer
>  */
> void tipc_bearer_xmit_skb(struct net *net, u32 bearer_id,
>diff --git a/net/tipc/bearer.h b/net/tipc/bearer.h
>index 490ad6e5f7a3..bd0cc5c287ef 100644
>--- a/net/tipc/bearer.h
>+++ b/net/tipc/bearer.h
>@@ -146,6 +146,7 @@ struct tipc_media {
>  * @identity: array index of this bearer within TIPC bearer array
>  * @disc: ptr to link setup request
>  * @net_plane: network plane ('A' through 'H') currently associated with bearer
>+ * @encap_hlen: encap headers length
>  * @up: bearer up flag (bit 0)
>  * @refcnt: tipc_bearer reference counter
>  *
>@@ -170,6 +171,7 @@ struct tipc_bearer {
> 	u32 identity;
> 	struct tipc_discoverer *disc;
> 	char net_plane;
>+	u16 encap_hlen;
> 	unsigned long up;
> 	refcount_t refcnt;
> };
>@@ -232,6 +234,7 @@ int tipc_bearer_setup(void);
> void tipc_bearer_cleanup(void);
> void tipc_bearer_stop(struct net *net);
> int tipc_bearer_mtu(struct net *net, u32 bearer_id);
>+int tipc_bearer_min_mtu(struct net *net, u32 bearer_id);
> bool tipc_bearer_bcast_support(struct net *net, u32 bearer_id);
> void tipc_bearer_xmit_skb(struct net *net, u32 bearer_id,
> 			  struct sk_buff *skb,
>diff --git a/net/tipc/udp_media.c b/net/tipc/udp_media.c
>index c2bb818704c8..0a85244fd618 100644
>--- a/net/tipc/udp_media.c
>+++ b/net/tipc/udp_media.c
>@@ -738,8 +738,8 @@ static int tipc_udp_enable(struct net *net, struct tipc_bearer *b,
> 			udp_conf.local_ip.s_addr = local.ipv4.s_addr;
> 		udp_conf.use_udp_checksums = false;
> 		ub->ifindex = dev->ifindex;
>-		if (tipc_mtu_bad(dev, sizeof(struct iphdr) +
>-				      sizeof(struct udphdr))) {
>+		b->encap_hlen = sizeof(struct iphdr) + sizeof(struct udphdr);
>+		if (tipc_mtu_bad(dev, b->encap_hlen)) {
> 			err = -EINVAL;
> 			goto err;
> 		}
>@@ -760,6 +760,7 @@ static int tipc_udp_enable(struct net *net, struct tipc_bearer *b,
> 		else
> 			udp_conf.local_ip6 = local.ipv6;
> 		ub->ifindex = dev->ifindex;
>+		b->encap_hlen = sizeof(struct ipv6hdr) + sizeof(struct udphdr);
tipc_mtu_bad() needs to be called here to check for the minimum required MTU like the way ipv4 UDP bearer does.
> 		b->mtu = 1280;
> #endif
> 	} else {
>--
>2.39.1
>
>
>
>_______________________________________________
>tipc-discussion mailing list
>tip...@li...
>https://lists.sourceforge.net/lists/listinfo/tipc-discussion

[tipc-discussion] [PATCH net 2/2] tipc: do not update mtu if msg_max is too small

[Xin L. <luc...@gm...>]

When doing link mtu negotiation, a malicious peer may send Activate msg
with a very small mtu, e.g. 4 in Shuang's testing, without checking for
the minimum mtu, l->mtu will be set to 4 in tipc_link_proto_rcv(), then
n->links[bearer_id].mtu is set to 4294967228, which is a overflow of
'4 - INT_H_SIZE - EMSG_OVERHEAD' in tipc_link_mss().

With tipc_link.mtu = 4, tipc_link_xmit() kept printing the warning:

 tipc: Too large msg, purging xmit list 1 5 0 40 4!
 tipc: Too large msg, purging xmit list 1 15 0 60 4!

And with tipc_link_entry.mtu 4294967228, a huge skb was allocated in
named_distribute(), and when purging it in tipc_link_xmit(), a crash
was even caused:

  general protection fault, probably for non-canonical address 0x2100001011000dd: 0000 [#1] PREEMPT SMP PTI
  CPU: 0 PID: 0 Comm: swapper/0 Kdump: loaded Not tainted 6.3.0.neta #19
  RIP: 0010:kfree_skb_list_reason+0x7e/0x1f0
  Call Trace:
   <IRQ>
   skb_release_data+0xf9/0x1d0
   kfree_skb_reason+0x40/0x100
   tipc_link_xmit+0x57a/0x740 [tipc]
   tipc_node_xmit+0x16c/0x5c0 [tipc]
   tipc_named_node_up+0x27f/0x2c0 [tipc]
   tipc_node_write_unlock+0x149/0x170 [tipc]
   tipc_rcv+0x608/0x740 [tipc]
   tipc_udp_recv+0xdc/0x1f0 [tipc]
   udp_queue_rcv_one_skb+0x33e/0x620
   udp_unicast_rcv_skb.isra.72+0x75/0x90
   __udp4_lib_rcv+0x56d/0xc20
   ip_protocol_deliver_rcu+0x100/0x2d0

This patch fixes it by checking the new mtu against tipc_bearer_min_mtu(),
and not updating mtu if it is too small.

Fixes: ed193ece2649 ("tipc: simplify link mtu negotiation")
Reported-by: Shuang Li <sh...@re...>
Signed-off-by: Xin Long <luc...@gm...>
---
 net/tipc/link.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/net/tipc/link.c b/net/tipc/link.c
index b3ce24823f50..a9e46c58b28a 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -2200,7 +2200,7 @@ static int tipc_link_proto_rcv(struct tipc_link *l, struct sk_buff *skb,
 	struct tipc_msg *hdr = buf_msg(skb);
 	struct tipc_gap_ack_blks *ga = NULL;
 	bool reply = msg_probe(hdr), retransmitted = false;
-	u32 dlen = msg_data_sz(hdr), glen = 0;
+	u32 dlen = msg_data_sz(hdr), glen = 0, msg_max;
 	u16 peers_snd_nxt =  msg_next_sent(hdr);
 	u16 peers_tol = msg_link_tolerance(hdr);
 	u16 peers_prio = msg_linkprio(hdr);
@@ -2283,8 +2283,9 @@ static int tipc_link_proto_rcv(struct tipc_link *l, struct sk_buff *skb,
 		l->peer_session = msg_session(hdr);
 		l->in_session = true;
 		l->peer_bearer_id = msg_bearer_id(hdr);
-		if (l->mtu > msg_max_pkt(hdr))
-			l->mtu = msg_max_pkt(hdr);
+		msg_max = msg_max_pkt(hdr);
+		if (msg_max >= tipc_bearer_min_mtu(l->net, l->bearer_id) && l->mtu > msg_max)
+			l->mtu = msg_max;
 		break;
 
 	case STATE_MSG:
-- 
2.39.1

[tipc-discussion] [PATCH net 1/2] tipc: add tipc_bearer_min_mtu to calculate min mtu

[Xin L. <luc...@gm...>]

As different media may requires different min mtu, and even the
same media with different net family requires different min mtu,
add tipc_bearer_min_mtu() to calculate min mtu accordingly.

This API will be used to check the new mtu when doing the link
mtu negotiation in the next patch.

Signed-off-by: Xin Long <luc...@gm...>
---
 net/tipc/bearer.c    | 13 +++++++++++++
 net/tipc/bearer.h    |  3 +++
 net/tipc/udp_media.c |  5 +++--
 3 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c
index 35cac7733fd3..c5d2e8c45f88 100644
--- a/net/tipc/bearer.c
+++ b/net/tipc/bearer.c
@@ -541,6 +541,19 @@ int tipc_bearer_mtu(struct net *net, u32 bearer_id)
 	return mtu;
 }
 
+int tipc_bearer_min_mtu(struct net *net, u32 bearer_id)
+{
+	int mtu = TIPC_MIN_BEARER_MTU;
+	struct tipc_bearer *b;
+
+	rcu_read_lock();
+	b = rcu_dereference(tipc_net(net)->bearer_list[bearer_id]);
+	if (b)
+		mtu += b->encap_hlen;
+	rcu_read_unlock();
+	return mtu;
+}
+
 /* tipc_bearer_xmit_skb - sends buffer to destination over bearer
  */
 void tipc_bearer_xmit_skb(struct net *net, u32 bearer_id,
diff --git a/net/tipc/bearer.h b/net/tipc/bearer.h
index 490ad6e5f7a3..bd0cc5c287ef 100644
--- a/net/tipc/bearer.h
+++ b/net/tipc/bearer.h
@@ -146,6 +146,7 @@ struct tipc_media {
  * @identity: array index of this bearer within TIPC bearer array
  * @disc: ptr to link setup request
  * @net_plane: network plane ('A' through 'H') currently associated with bearer
+ * @encap_hlen: encap headers length
  * @up: bearer up flag (bit 0)
  * @refcnt: tipc_bearer reference counter
  *
@@ -170,6 +171,7 @@ struct tipc_bearer {
 	u32 identity;
 	struct tipc_discoverer *disc;
 	char net_plane;
+	u16 encap_hlen;
 	unsigned long up;
 	refcount_t refcnt;
 };
@@ -232,6 +234,7 @@ int tipc_bearer_setup(void);
 void tipc_bearer_cleanup(void);
 void tipc_bearer_stop(struct net *net);
 int tipc_bearer_mtu(struct net *net, u32 bearer_id);
+int tipc_bearer_min_mtu(struct net *net, u32 bearer_id);
 bool tipc_bearer_bcast_support(struct net *net, u32 bearer_id);
 void tipc_bearer_xmit_skb(struct net *net, u32 bearer_id,
 			  struct sk_buff *skb,
diff --git a/net/tipc/udp_media.c b/net/tipc/udp_media.c
index c2bb818704c8..0a85244fd618 100644
--- a/net/tipc/udp_media.c
+++ b/net/tipc/udp_media.c
@@ -738,8 +738,8 @@ static int tipc_udp_enable(struct net *net, struct tipc_bearer *b,
 			udp_conf.local_ip.s_addr = local.ipv4.s_addr;
 		udp_conf.use_udp_checksums = false;
 		ub->ifindex = dev->ifindex;
-		if (tipc_mtu_bad(dev, sizeof(struct iphdr) +
-				      sizeof(struct udphdr))) {
+		b->encap_hlen = sizeof(struct iphdr) + sizeof(struct udphdr);
+		if (tipc_mtu_bad(dev, b->encap_hlen)) {
 			err = -EINVAL;
 			goto err;
 		}
@@ -760,6 +760,7 @@ static int tipc_udp_enable(struct net *net, struct tipc_bearer *b,
 		else
 			udp_conf.local_ip6 = local.ipv6;
 		ub->ifindex = dev->ifindex;
+		b->encap_hlen = sizeof(struct ipv6hdr) + sizeof(struct udphdr);
 		b->mtu = 1280;
 #endif
 	} else {
-- 
2.39.1

[tipc-discussion] [PATCH net 0/2] tipc: fix the mtu update in link mtu negotiation

[Xin L. <luc...@gm...>]

This patchset fixes a crash caused by a too small MTU carried in the
activate msg. Note that as such malicious packet does not exist in
the normal env, and the fix won't break any application.

The 1st patch introduces a function to calculate the minimum MTU for
the bearer, and the 2nd patch fixes the crash with this function.

Xin Long (2):
  tipc: add tipc_bearer_min_mtu to calculate min mtu
  tipc: do not update mtu if msg_max is too small

 net/tipc/bearer.c    | 13 +++++++++++++
 net/tipc/bearer.h    |  3 +++
 net/tipc/link.c      |  7 ++++---
 net/tipc/udp_media.c |  5 +++--
 4 files changed, 23 insertions(+), 5 deletions(-)

-- 
2.39.1

Re: [tipc-discussion] TIPC socket ( SOCK_SEQPACKET) cleanup issue

[prakash b. <ps1...@gm...>]

Thanks Tung.
Please find my response inline.


Thanks and regards,
Prakash

On Tue, Apr 18, 2023 at 7:04 AM Tung Quang Nguyen <
tun...@de...> wrote:

>
>
>
>
> *From:* prakash bisht <ps1...@gm...>
> *Sent:* Monday, April 17, 2023 11:20 AM
> *To:* tip...@li...; Xin Long <lx...@re...>;
> Tung Quang Nguyen <tun...@de...>; jm...@re...
> *Subject:* Fwd: TIPC socket ( SOCK_SEQPACKET) cleanup issue
>
>
>
> Hi John/Xin,Tung,
>
>
>
> Any thoughts on my previous email? We are using tipc for our product for
> quite a while and started facing this issue recently in a specific scenario
> we launch strace to monitor another process.
>
> Also is there any way to deny creation of another tipc socket with the
> same tipc address ?  In our case applications need a unique tipc server
> socket.
>
> >>> There is no limitation for creating many sockets binding to the same
> tipc address. Why you need “a unique tipc server socket” ? Can you provide
> your code to demonstrate your use case ?
>


> *[Prakash]* We have services which are unique in our system and not meant
> for load balancing. Each service is uniquely represented by a tipc service
> address(type,instance).  What we are looking for is that if someone by
> mistake creates a second Service using the same address(type,instance) then
> the service creation fail and throw an error. This is just to avoid a
> silent failure.
>



>
>
> Thanks and Regards,
>
> Prakash
>
> ---------- Forwarded message ---------
> From: *prakash bisht* <ps1...@gm...>
> Date: Mon, Apr 3, 2023 at 4:33 PM
> Subject: TIPC socket ( SOCK_SEQPACKET) cleanup issue
> To: <tip...@li...>
>
>
>
> Hi all,
>
>
>
> I am facing an issue while closing the TIPC server socket. In certain
> scenarios, even after closing the server socket fd the ‘tipc socket list’
> is still showing it as alive.
>
> >>> What is you iproute2 version ? Can you provide your code to
> demonstrate your use case ?
>


> *[Prakash ]*  We are using iproute2 version-4.20.0-2+deb10u1 on amd64
> platform. Our use case is very simple. We are creating/destroying a server
> socket based on some event using below code.
>
      // server socket creation
           int sd = socket(AF_TIPC, SOCK_SEQPACKET, 0);
      // Closing server socket
          close(sd);

      After closing the socket the file descriptor is freed but the tipc
socket is still present in "tipc socket list" output.
      We have multiple applications in our system which are using tipc
sockets. But we see this behaviour(stale tipc socket) only in one
application.
      The only difference which I can notice is that this particular
application  is spawning "strace" to monitor some other application. I am
not sure but it looks like somehow running strace is affecting tipc socket

       cleanup.



> I am sure that the fd has been closed as the next socket creation request
> gets the same fd from linux. Even when the process exits, the stale socket
> entry is still present in the ‘tipc socket list’ and it vanishes only after
> rebooting the system.
>
>  Kernel version : 4.19.81
>
> Socket type :      SOCK_SEQPACKET
>
>
>
> Also, is there any way of finding out whether a tipc socket belongs to
> which linux process ?
>
> >>> There is no command to know which tipc socket belongs to which Linux
> process. But you can use function getsockname() to print out the port id
> after creating a socket. Then you know the created socket belongs to the
> calling process.
>
[

>  *[Prakash]* Thanks.  getsockname() should server the purpose here.
>


> Would appreciate any help.
>
>
>
> Thanks,
>
> Prakash
>

Re: [tipc-discussion] TIPC socket ( SOCK_SEQPACKET) cleanup issue

[Tung Q. N. <tun...@de...>]

From: prakash bisht <ps1...@gm...>
Sent: Monday, April 17, 2023 11:20 AM
To: tip...@li...; Xin Long <lx...@re...>; Tung Quang Nguyen <tun...@de...>; jm...@re...
Subject: Fwd: TIPC socket ( SOCK_SEQPACKET) cleanup issue

Hi John/Xin,Tung,

Any thoughts on my previous email? We are using tipc for our product for quite a while and started facing this issue recently in a specific scenario we launch strace to monitor another process.
Also is there any way to deny creation of another tipc socket with the same tipc address ?  In our case applications need a unique tipc server socket.
>>> There is no limitation for creating many sockets binding to the same tipc address. Why you need “a unique tipc server socket” ? Can you provide your code to demonstrate your use case ?

Thanks and Regards,
Prakash
---------- Forwarded message ---------
From: prakash bisht <ps1...@gm...<mailto:ps1...@gm...>>
Date: Mon, Apr 3, 2023 at 4:33 PM
Subject: TIPC socket ( SOCK_SEQPACKET) cleanup issue
To: <tip...@li...<mailto:tip...@li...>>

Hi all,

I am facing an issue while closing the TIPC server socket. In certain scenarios, even after closing the server socket fd the ‘tipc socket list’ is still showing it as alive.
>>> What is you iproute2 version ? Can you provide your code to demonstrate your use case ?
I am sure that the fd has been closed as the next socket creation request gets the same fd from linux. Even when the process exits, the stale socket entry is still present in the ‘tipc socket list’ and it vanishes only after rebooting the system.
 Kernel version : 4.19.81
Socket type :      SOCK_SEQPACKET

Also, is there any way of finding out whether a tipc socket belongs to which linux process ?
>>> There is no command to know which tipc socket belongs to which Linux process. But you can use function getsockname() to print out the port id after creating a socket. Then you know the created socket belongs to the calling process.

Would appreciate any help.

Thanks,
Prakash

[tipc-discussion] Fwd: TIPC socket ( SOCK_SEQPACKET) cleanup issue

[prakash b. <ps1...@gm...>]

Hi John/Xin,Tung,

Any thoughts on my previous email? We are using tipc for our product for
quite a while and started facing this issue recently in a specific scenario
we launch strace to monitor another process.
Also is there any way to deny creation of another tipc socket with the same
tipc address ?  In our case applications need a unique tipc server socket.

Thanks and Regards,
Prakash

---------- Forwarded message ---------
From: prakash bisht <ps1...@gm...>
Date: Mon, Apr 3, 2023 at 4:33 PM
Subject: TIPC socket ( SOCK_SEQPACKET) cleanup issue
To: <tip...@li...>


Hi all,



I am facing an issue while closing the TIPC server socket. In certain
scenarios, even after closing the server socket fd the ‘tipc socket list’
is still showing it as alive.

I am sure that the fd has been closed as the next socket creation request
gets the same fd from linux. Even when the process exits, the stale socket
entry is still present in the ‘tipc socket list’ and it vanishes only after
rebooting the system.

 Kernel version : 4.19.81

Socket type :      SOCK_SEQPACKET



Also, is there any way of finding out whether a tipc socket belongs to
which linux process ?



Would appreciate any help.


Thanks,

Prakash

Re: [tipc-discussion] TIPC issue

[Jon M. <jm...@re...>]

On 2023-04-04 20:30, Jon Maloy wrote:
>
>
> On 2023-03-16 12:03, Nagendra Kumar via tipc-discussion wrote:
>>   Hi Jon/Tuong/Tung/Hoang/Thang,Is there any thoughts on the below 
>> email trails??
>> Thanks-Nagendra
>>      On Monday, 13 March, 2023 at 10:59:45 pm IST, Nagendra Kumar via 
>> tipc-discussion <tip...@li...> wrote:
>>      Sending it again.....
>>      On Friday, 10 March, 2023 at 11:49:59 am IST, Nagendra Kumar 
>> <nag...@ya...> wrote:
>>     Hi,We are trying to use TIPC on RHEL8.4 to manually communicate 
>> OpenSAF nodes, using TIPC instead of TCP.
>> OpenSAF is designed to work with TIPC but only as L2 and, in this 
>> case, we need IP routing. That's why we are configuring it manually.
>> I am using the following script to start and configure TIPC:#!/bin/bash
>> SLOT_ID=$(cat "/etc/opensaf/slot_id")DEV=eno1
>> modprobe tipc
>
>> tipc node set netid 1111tipc node set address 1.1.$SLOT_IDtipc node 
>> set identity $(hostname)tipc bearer enable media udp device $DEV name 
>> $(hostname)tipc media set mtu 9000 media udp
>> (Configuring TIPC with UDP we get TIPC traffic between nodes of 
>> different cabinets)
>>   They have all run the same script. Sometimes it happens to some and 
>> sometimes it happens to others doing exactly the same. In this case 
>> procs and ssaf(pics attached) are in different VLANs. When they are 
>> in the same VLAN, they always work correctly.
>>
> I notice that you are setting both 'tipc node set address' and 'tipc 
> node set identity'.  You only set one or the other, never both, since 
> setting an address will create an identity and vice versa.
> This should not really cause any trouble, -the value you set first 
> will cause that the second one will be ignored.
> I will still recommend that you remove one of those and try again, 
> before I spend and more time on this.
>
> Cheers
> ///jon

Hi again,
There is a commit upstream, commit 
c244c092f1ed2acfb5af3d3da81e22367d3dd733 ("tipc: fix unexpected link 
reset due to discovery messages") that is fixing an issue with link 
dicovery that seems related.
It has not yet been backported to rhel-8.4, but it can be cherry-picked 
cleanly if you want to try.
It all depends on whether you build own kernels, of course...

Regards
///jon


>
>>
>> I don't know if it's a network or software problem as communications 
>> are working fine.
>> RHEL version: Red Hat Enterprise Linux release 8.4 (Ootpa)
>> Kernel version: 4.18.0-305.el8.x86_64
>> TIPC version: Built-in kernel module
>>
>>
>>       Thank you !-Nagendra
>>    _______________________________________________
>> tipc-discussion mailing list
>> tip...@li...
>> https://lists.sourceforge.net/lists/listinfo/tipc-discussion
>>    _______________________________________________
>> tipc-discussion mailing list
>> tip...@li...
>> https://lists.sourceforge.net/lists/listinfo/tipc-discussion
>

Re: [tipc-discussion] TIPC issue

[Jon M. <jm...@re...>]

On 2023-03-16 12:03, Nagendra Kumar via tipc-discussion wrote:
>   Hi Jon/Tuong/Tung/Hoang/Thang,Is there any thoughts on the below email trails??
> Thanks-Nagendra
>      On Monday, 13 March, 2023 at 10:59:45 pm IST, Nagendra Kumar via tipc-discussion <tip...@li...> wrote:
>   
>    Sending it again.....
>      On Friday, 10 March, 2023 at 11:49:59 am IST, Nagendra Kumar <nag...@ya...> wrote:
>   
>   Hi,We are trying to use TIPC on RHEL8.4 to manually communicate OpenSAF nodes, using TIPC instead of TCP.
> OpenSAF is designed to work with TIPC but only as L2 and, in this case, we need IP routing. That's why we are configuring it manually.
> I am using the following script to start and configure TIPC:#!/bin/bash
> SLOT_ID=$(cat "/etc/opensaf/slot_id")DEV=eno1
> modprobe tipc

> tipc node set netid 1111tipc node set address 1.1.$SLOT_IDtipc node set identity $(hostname)tipc bearer enable media udp device $DEV name $(hostname)tipc media set mtu 9000 media udp
> (Configuring TIPC with UDP we get TIPC traffic between nodes of different cabinets)
>   They have all run the same script. Sometimes it happens to some and sometimes it happens to others doing exactly the same. In this case procs and ssaf(pics attached) are in different VLANs. When they are in the same VLAN, they always work correctly.
>
I notice that you are setting both 'tipc node set address' and 'tipc 
node set identity'.  You only set one or the other, never both, since 
setting an address will create an identity and vice versa.
This should not really cause any trouble, -the value you set first will 
cause that the second one will be ignored.
I will still recommend that you remove one of those and try again, 
before I spend and more time on this.

Cheers
///jon

>
> I don't know if it's a network or software problem as communications are working fine.
> RHEL version: Red Hat Enterprise Linux release 8.4 (Ootpa)
> Kernel version: 4.18.0-305.el8.x86_64
> TIPC version: Built-in kernel module
>
>
>     
>   Thank you !-Nagendra
>    
> _______________________________________________
> tipc-discussion mailing list
> tip...@li...
> https://lists.sourceforge.net/lists/listinfo/tipc-discussion
>    
> _______________________________________________
> tipc-discussion mailing list
> tip...@li...
> https://lists.sourceforge.net/lists/listinfo/tipc-discussion

Re: [tipc-discussion] Requesting Support On TIPC TIPC_ERR_NO_PORT

[Jon M. <jm...@re...>]

On 2023-03-22 06:48, Ragavendran Sridharan wrote:
> Hi Team,
>
> I am requesting assistance onTIPC in linux. In our project we are using
> Tipc version 2.02 version .  We are facing the issue of TIPC connections
> not getting establised and returning Error as :TIPC_ERR_NO_PORT from TIPC
> Server.
>
> Could you please list down the scenarios in which this errors will occur.
If you are using a tipc_service_addr (called tipc_port_name in older 
versions) as destination address you should get a TIPC_ERR_NO_NAME if 
the server socket does not exist.
So, I conclude that you are trying to do connect using a 
tipc_socket_addr (aka tipc_port_id in older versions), which would give 
this error.

You should look into how you have obtained your server socket address, 
and that this has not been garbled when you transferred it to the client 
socket.
note that a tipc_socket_address is volatile, and only is valid during 
the existence of the particular socket you are trying to connect to.

I hope this helps.

///jon

>
> Thank And Regards,
> Raagavendran
>
>
> <https://docs.huihoo.com/doxygen/linux/kernel/3.7/tipc_8h.html#a0cba261c068b96e6f296218445f75f78>
>
> _______________________________________________
> tipc-discussion mailing list
> tip...@li...
> https://lists.sourceforge.net/lists/listinfo/tipc-discussion
>

[tipc-discussion] TIPC socket ( SOCK_SEQPACKET) cleanup issue

[prakash b. <ps1...@gm...>]

Hi all,



I am facing an issue while closing the TIPC server socket. In certain
scenarios, even after closing the server socket fd the ‘tipc socket list’
is still showing it as alive.

I am sure that the fd has been closed as the next socket creation request
gets the same fd from linux. Even when the process exits, the stale socket
entry is still present in the ‘tipc socket list’ and it vanishes only after
rebooting the system.

 Kernel version : 4.19.81

Socket type :      SOCK_SEQPACKET



Also, is there any way of finding out whether a tipc socket belongs to
which linux process ?



Would appreciate any help.


Thanks,

Prakash

Re: [tipc-discussion] Requesting Support On TIPC TIPC_ERR_NO_PORT

[Jon M. <jm...@re...>]

Hi,
We are not using versioning of TIPC anymore; it follows the Linux kernel 
version where it is embedded.
Please provide the Linux version you are using when you encounter this 
problem.

We also need to know the network/node /interface setup you are using and 
the commands you are giving to TIPC to set up TIPC.

Thne I hope we can get some further.

Regards
Jon Maloy

On 2023-03-22 06:48, Ragavendran Sridharan wrote:
> Hi Team,
>
> I am requesting assistance onTIPC in linux. In our project we are using
> Tipc version 2.02 version .  We are facing the issue of TIPC connections
> not getting establised and returning Error as :TIPC_ERR_NO_PORT from TIPC
> Server.
>
> Could you please list down the scenarios in which this errors will occur.
>
> Thank And Regards,
> Raagavendran
>
>
> <https://docs.huihoo.com/doxygen/linux/kernel/3.7/tipc_8h.html#a0cba261c068b96e6f296218445f75f78>
>
> _______________________________________________
> tipc-discussion mailing list
> tip...@li...
> https://lists.sourceforge.net/lists/listinfo/tipc-discussion
>

Re: [tipc-discussion] TIPC issue

[Jon M. <jm...@re...>]

Hi Nagendra,
Sorry for not noticing this earlier. I will look into this next week.

///jon

On 2023-03-13 12:58, Nagendra Kumar via tipc-discussion wrote:
>   Sending it again.....
>      On Friday, 10 March, 2023 at 11:49:59 am IST, Nagendra Kumar <nag...@ya...> wrote:
>   
>   Hi,We are trying to use TIPC on RHEL8.4 to manually communicate OpenSAF nodes, using TIPC instead of TCP.
> OpenSAF is designed to work with TIPC but only as L2 and, in this case, we need IP routing. That's why we are configuring it manually.
> I am using the following script to start and configure TIPC:#!/bin/bash
> SLOT_ID=$(cat "/etc/opensaf/slot_id")DEV=eno1
> modprobe tipctipc node set netid 1111tipc node set address 1.1.$SLOT_IDtipc node set identity $(hostname)tipc bearer enable media udp device $DEV name $(hostname)tipc media set mtu 9000 media udp
> (Configuring TIPC with UDP we get TIPC traffic between nodes of different cabinets)
>   They have all run the same script. Sometimes it happens to some and sometimes it happens to others doing exactly the same. In this case procs and ssaf(pics attached) are in different VLANs. When they are in the same VLAN, they always work correctly.
>
>
>
> I don't know if it's a network or software problem as communications are working fine.
> RHEL version: Red Hat Enterprise Linux release 8.4 (Ootpa)
> Kernel version: 4.18.0-305.el8.x86_64
> TIPC version: Built-in kernel module

>
>
>     
>   Thank you !-Nagendra
>    
> _______________________________________________
> tipc-discussion mailing list
> tip...@li...
> https://lists.sourceforge.net/lists/listinfo/tipc-discussion

[tipc-discussion] Requesting Support On TIPC TIPC_ERR_NO_PORT

[Ragavendran S. <rag...@gm...>]

Hi Team,

I am requesting assistance onTIPC in linux. In our project we are using
Tipc version 2.02 version .  We are facing the issue of TIPC connections
not getting establised and returning Error as :TIPC_ERR_NO_PORT from TIPC
Server.

Could you please list down the scenarios in which this errors will occur.

Thank And Regards,
Raagavendran


<https://docs.huihoo.com/doxygen/linux/kernel/3.7/tipc_8h.html#a0cba261c068b96e6f296218445f75f78>

Re: [tipc-discussion] TIPC issue

[Nagendra K. <nag...@ya...>]

 Hi Jon/Tuong/Tung/Hoang/Thang,Is there any thoughts on the below email trails??
Thanks-Nagendra
    On Monday, 13 March, 2023 at 10:59:45 pm IST, Nagendra Kumar via tipc-discussion <tip...@li...> wrote:  
 
  Sending it again.....
    On Friday, 10 March, 2023 at 11:49:59 am IST, Nagendra Kumar <nag...@ya...> wrote:  
 
 Hi,We are trying to use TIPC on RHEL8.4 to manually communicate OpenSAF nodes, using TIPC instead of TCP.
OpenSAF is designed to work with TIPC but only as L2 and, in this case, we need IP routing. That's why we are configuring it manually.
I am using the following script to start and configure TIPC:#!/bin/bash
SLOT_ID=$(cat "/etc/opensaf/slot_id")DEV=eno1
modprobe tipctipc node set netid 1111tipc node set address 1.1.$SLOT_IDtipc node set identity $(hostname)tipc bearer enable media udp device $DEV name $(hostname)tipc media set mtu 9000 media udp
(Configuring TIPC with UDP we get TIPC traffic between nodes of different cabinets)
 They have all run the same script. Sometimes it happens to some and sometimes it happens to others doing exactly the same. In this case procs and ssaf(pics attached) are in different VLANs. When they are in the same VLAN, they always work correctly.



I don't know if it's a network or software problem as communications are working fine.
RHEL version: Red Hat Enterprise Linux release 8.4 (Ootpa)
Kernel version: 4.18.0-305.el8.x86_64
TIPC version: Built-in kernel module


   
 Thank you !-Nagendra
  
_______________________________________________
tipc-discussion mailing list
tip...@li...
https://lists.sourceforge.net/lists/listinfo/tipc-discussion
  

Re: [tipc-discussion] TIPC issue

[Nagendra K. <nag...@ya...>]

 Sending it again.....
    On Friday, 10 March, 2023 at 11:49:59 am IST, Nagendra Kumar <nag...@ya...> wrote:  
 
 Hi,We are trying to use TIPC on RHEL8.4 to manually communicate OpenSAF nodes, using TIPC instead of TCP.
OpenSAF is designed to work with TIPC but only as L2 and, in this case, we need IP routing. That's why we are configuring it manually.
I am using the following script to start and configure TIPC:#!/bin/bash
SLOT_ID=$(cat "/etc/opensaf/slot_id")DEV=eno1
modprobe tipctipc node set netid 1111tipc node set address 1.1.$SLOT_IDtipc node set identity $(hostname)tipc bearer enable media udp device $DEV name $(hostname)tipc media set mtu 9000 media udp
(Configuring TIPC with UDP we get TIPC traffic between nodes of different cabinets)
 They have all run the same script. Sometimes it happens to some and sometimes it happens to others doing exactly the same. In this case procs and ssaf(pics attached) are in different VLANs. When they are in the same VLAN, they always work correctly.



I don't know if it's a network or software problem as communications are working fine.
RHEL version: Red Hat Enterprise Linux release 8.4 (Ootpa)
Kernel version: 4.18.0-305.el8.x86_64
TIPC version: Built-in kernel module


   
 Thank you !-Nagendra
  

Re: [tipc-discussion] [PATCH v1 1/1] tipc: fix kernel warning when sending SYN message

[Jon M. <jm...@re...>]

On 2023-02-05 23:11, Tung Quang Nguyen wrote:
>
>> -----Original Message-----
>> From: Jon Maloy <jm...@re...>
>> Sent: Saturday, February 4, 2023 1:02 AM
>> To: Tung Quang Nguyen <tun...@de...>; tip...@li...; ma...@do...;
>> yin...@wi...
>> Subject: Re: [tipc-discussion][PATCH v1 1/1] tipc: fix kernel warning when sending SYN message
>>
>>
>>
>> On 2023-01-31 23:03, Tung Nguyen wrote:
>>> When sending a SYN message, this kernel stack trace is observed:
>>>
>>> ...
>>> [   13.396352] RIP: 0010:_copy_from_iter+0xb4/0x550
>>> ...
>>> [   13.398494] Call Trace:
>>> [   13.398630]  <TASK>
>>> [   13.398630]  ? __alloc_skb+0xed/0x1a0
>>> [   13.398630]  tipc_msg_build+0x12c/0x670 [tipc]
>>> [   13.398630]  ? shmem_add_to_page_cache.isra.71+0x151/0x290
>>> [   13.398630]  __tipc_sendmsg+0x2d1/0x710 [tipc]
>>> [   13.398630]  ? tipc_connect+0x1d9/0x230 [tipc]
>>> [   13.398630]  ? __local_bh_enable_ip+0x37/0x80
>>> [   13.398630]  tipc_connect+0x1d9/0x230 [tipc]
>>> [   13.398630]  ? __sys_connect+0x9f/0xd0
>>> [   13.398630]  __sys_connect+0x9f/0xd0
>>> [   13.398630]  ? preempt_count_add+0x4d/0xa0
>>> [   13.398630]  ? fpregs_assert_state_consistent+0x22/0x50
>>> [   13.398630]  __x64_sys_connect+0x16/0x20
>>> [   13.398630]  do_syscall_64+0x42/0x90
>>> [   13.398630]  entry_SYSCALL_64_after_hwframe+0x63/0xcd
>>>
>>> It is because commit a41dad905e5a ("iov_iter: saner checks for attempt to copy to/from iterator")
>>> has introduced sanity check for copying from/to iov iterator. Lacking
>>> of copy direction from the iterator viewpoint would lead to kernel
>>> stack trace like above.
>>>
>>> This commit fixes this issue by initializing the iov iterator with
>>> the correct copy direction.
>>>
>>> Signed-off-by: Tung Nguyen <tun...@de...>
>>> ---
>>>    net/tipc/msg.c | 4 ++++
>>>    1 file changed, 4 insertions(+)
>>>
>>> diff --git a/net/tipc/msg.c b/net/tipc/msg.c
>>> index 5c9fd4791c4b..f40cd9032b62 100644
>>> --- a/net/tipc/msg.c
>>> +++ b/net/tipc/msg.c
>>> @@ -377,10 +377,14 @@ int tipc_msg_build(struct tipc_msg *mhdr, struct msghdr *m, int offset,
>>>    	int pktno = 1;
>>>    	char *pktpos;
>>>    	int pktsz;
>>> +	struct iovec iov;
>>>    	int rc;
>>>
>>>    	msg_set_size(mhdr, msz);
>>>
>>> +	if (!dsz)
>>> +		iov_iter_init(&m->msg_iter, ITER_SOURCE, &iov, 0, 0);
>>> +
>>>    	/* No fragmentation needed? */
>>>    	if (likely(msz <= pktmax)) {
>>>    		skb = tipc_buf_acquire(msz, GFP_KERNEL);
>> I feel a little uncomfortable with adding an uninitialized struct iovec
>> here.
> It is OK not using iovec, just passing NULL to iov_iter_init: iov_iter_init(&m->msg_iter, ITER_SOURCE, NULL, 0, 0).
> My intention was to highlight the copy source and no-copy (count = 0) information.
>> Al Viro had a different proposal in his email from Dec 8th:
>>
>> On 2022-12-11 04:38, Al Viro wrote:
>>> On Thu, Dec 08, 2022 at 08:38:14PM +0100, Eric Dumazet wrote:
>>>
>>>> Exposes an old bug in tipc ?
>>>>
>>>> Seems a new check added by Al in :
>>>>
>>>> Author: Al Viro <vi...@ze...>
>>>> Date:   Thu Sep 15 20:11:15 2022 -0400
>>>>
>>>>       iov_iter: saner checks for attempt to copy to/from iterator
>>>>
>>>>       instead of "don't do it to ITER_PIPE" check for ->data_source being
>>>>       false on copying from iterator.  Check for !->data_source for
>>>>       copying to iterator, while we are at it.
>>>>
>>>>       Signed-off-by: Al Viro <vi...@ze...>
>>> Lovely...  zero-length sendmsg with uninitialized ->msg_data...
>>>
>>> 	I would probably argue that it's a bug in tipc_connect(),
>>> fixed by iov_iter_kvec(&m.msg_iter, ITER_SOURCE, NULL, 0, 0);
>>> in there.  Depends - if that kind of uninitialized msg_iter used
>>> as zero length source or zero length destination is a frequent pattern,
>>> might as well make zero-byte copy_...iter() succeed quietly;
>>> I hope it isn't, but that's definitely something I'd missed
>>> when doing that series.
>>>
>>> 	I'll take a look tomorrow^Win the morning, after I get
>>> some sleep...
>>>
>> Did you consider that one?
> This function has the same effect as the one I used. But from the viewpoint of function
> tipc_msg_build(), we mainly want to copy data from user-space. Two exceptions are SYN and ACK- with no data. So, I think using iov_iter_init() makes more sense.
> I suggest that we go with what I did (plus passing NULL for iovec) and CC Al Viro to see if the guy has any objection.
> What do you think ?
Yes, that makes sense. Go ahead.

///jon

>> ///jon

Re: [tipc-discussion] [PATCH v1 1/1] tipc: fix kernel warning when sending SYN message

[Tung Q. N. <tun...@de...>]

>-----Original Message-----
>From: Jon Maloy <jm...@re...>
>Sent: Saturday, February 4, 2023 1:02 AM
>To: Tung Quang Nguyen <tun...@de...>; tip...@li...; ma...@do...;
>yin...@wi...
>Subject: Re: [tipc-discussion][PATCH v1 1/1] tipc: fix kernel warning when sending SYN message
>
>
>
>On 2023-01-31 23:03, Tung Nguyen wrote:
>> When sending a SYN message, this kernel stack trace is observed:
>>
>> ...
>> [   13.396352] RIP: 0010:_copy_from_iter+0xb4/0x550
>> ...
>> [   13.398494] Call Trace:
>> [   13.398630]  <TASK>
>> [   13.398630]  ? __alloc_skb+0xed/0x1a0
>> [   13.398630]  tipc_msg_build+0x12c/0x670 [tipc]
>> [   13.398630]  ? shmem_add_to_page_cache.isra.71+0x151/0x290
>> [   13.398630]  __tipc_sendmsg+0x2d1/0x710 [tipc]
>> [   13.398630]  ? tipc_connect+0x1d9/0x230 [tipc]
>> [   13.398630]  ? __local_bh_enable_ip+0x37/0x80
>> [   13.398630]  tipc_connect+0x1d9/0x230 [tipc]
>> [   13.398630]  ? __sys_connect+0x9f/0xd0
>> [   13.398630]  __sys_connect+0x9f/0xd0
>> [   13.398630]  ? preempt_count_add+0x4d/0xa0
>> [   13.398630]  ? fpregs_assert_state_consistent+0x22/0x50
>> [   13.398630]  __x64_sys_connect+0x16/0x20
>> [   13.398630]  do_syscall_64+0x42/0x90
>> [   13.398630]  entry_SYSCALL_64_after_hwframe+0x63/0xcd
>>
>> It is because commit a41dad905e5a ("iov_iter: saner checks for attempt to copy to/from iterator")
>> has introduced sanity check for copying from/to iov iterator. Lacking
>> of copy direction from the iterator viewpoint would lead to kernel
>> stack trace like above.
>>
>> This commit fixes this issue by initializing the iov iterator with
>> the correct copy direction.
>>
>> Signed-off-by: Tung Nguyen <tun...@de...>
>> ---
>>   net/tipc/msg.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/net/tipc/msg.c b/net/tipc/msg.c
>> index 5c9fd4791c4b..f40cd9032b62 100644
>> --- a/net/tipc/msg.c
>> +++ b/net/tipc/msg.c
>> @@ -377,10 +377,14 @@ int tipc_msg_build(struct tipc_msg *mhdr, struct msghdr *m, int offset,
>>   	int pktno = 1;
>>   	char *pktpos;
>>   	int pktsz;
>> +	struct iovec iov;
>>   	int rc;
>>
>>   	msg_set_size(mhdr, msz);
>>
>> +	if (!dsz)
>> +		iov_iter_init(&m->msg_iter, ITER_SOURCE, &iov, 0, 0);
>> +
>>   	/* No fragmentation needed? */
>>   	if (likely(msz <= pktmax)) {
>>   		skb = tipc_buf_acquire(msz, GFP_KERNEL);
>
>I feel a little uncomfortable with adding an uninitialized struct iovec
>here.
It is OK not using iovec, just passing NULL to iov_iter_init: iov_iter_init(&m->msg_iter, ITER_SOURCE, NULL, 0, 0).
My intention was to highlight the copy source and no-copy (count = 0) information.
>
>Al Viro had a different proposal in his email from Dec 8th:
>
>On 2022-12-11 04:38, Al Viro wrote:
>> On Thu, Dec 08, 2022 at 08:38:14PM +0100, Eric Dumazet wrote:
>>
>>> Exposes an old bug in tipc ?
>>>
>>> Seems a new check added by Al in :
>>>
>>> Author: Al Viro <vi...@ze...>
>>> Date:   Thu Sep 15 20:11:15 2022 -0400
>>>
>>>      iov_iter: saner checks for attempt to copy to/from iterator
>>>
>>>      instead of "don't do it to ITER_PIPE" check for ->data_source being
>>>      false on copying from iterator.  Check for !->data_source for
>>>      copying to iterator, while we are at it.
>>>
>>>      Signed-off-by: Al Viro <vi...@ze...>
>> Lovely...  zero-length sendmsg with uninitialized ->msg_data...
>>
>> 	I would probably argue that it's a bug in tipc_connect(),
>> fixed by iov_iter_kvec(&m.msg_iter, ITER_SOURCE, NULL, 0, 0);
>> in there.  Depends - if that kind of uninitialized msg_iter used
>> as zero length source or zero length destination is a frequent pattern,
>> might as well make zero-byte copy_...iter() succeed quietly;
>> I hope it isn't, but that's definitely something I'd missed
>> when doing that series.
>>
>> 	I'll take a look tomorrow^Win the morning, after I get
>> some sleep...
>>
>
>Did you consider that one?
This function has the same effect as the one I used. But from the viewpoint of function
tipc_msg_build(), we mainly want to copy data from user-space. Two exceptions are SYN and ACK- with no data. So, I think using iov_iter_init() makes more sense.
I suggest that we go with what I did (plus passing NULL for iovec) and CC Al Viro to see if the guy has any objection.
What do you think ?
>
>///jon

Re: [tipc-discussion] [PATCH v1 1/1] tipc: fix kernel warning when sending SYN message

[Jon M. <jm...@re...>]

On 2023-01-31 23:03, Tung Nguyen wrote:
> When sending a SYN message, this kernel stack trace is observed:
>
> ...
> [   13.396352] RIP: 0010:_copy_from_iter+0xb4/0x550
> ...
> [   13.398494] Call Trace:
> [   13.398630]  <TASK>
> [   13.398630]  ? __alloc_skb+0xed/0x1a0
> [   13.398630]  tipc_msg_build+0x12c/0x670 [tipc]
> [   13.398630]  ? shmem_add_to_page_cache.isra.71+0x151/0x290
> [   13.398630]  __tipc_sendmsg+0x2d1/0x710 [tipc]
> [   13.398630]  ? tipc_connect+0x1d9/0x230 [tipc]
> [   13.398630]  ? __local_bh_enable_ip+0x37/0x80
> [   13.398630]  tipc_connect+0x1d9/0x230 [tipc]
> [   13.398630]  ? __sys_connect+0x9f/0xd0
> [   13.398630]  __sys_connect+0x9f/0xd0
> [   13.398630]  ? preempt_count_add+0x4d/0xa0
> [   13.398630]  ? fpregs_assert_state_consistent+0x22/0x50
> [   13.398630]  __x64_sys_connect+0x16/0x20
> [   13.398630]  do_syscall_64+0x42/0x90
> [   13.398630]  entry_SYSCALL_64_after_hwframe+0x63/0xcd
>
> It is because commit a41dad905e5a ("iov_iter: saner checks for attempt to copy to/from iterator")
> has introduced sanity check for copying from/to iov iterator. Lacking
> of copy direction from the iterator viewpoint would lead to kernel
> stack trace like above.
>
> This commit fixes this issue by initializing the iov iterator with
> the correct copy direction.
>
> Signed-off-by: Tung Nguyen <tun...@de...>
> ---
>   net/tipc/msg.c | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/net/tipc/msg.c b/net/tipc/msg.c
> index 5c9fd4791c4b..f40cd9032b62 100644
> --- a/net/tipc/msg.c
> +++ b/net/tipc/msg.c
> @@ -377,10 +377,14 @@ int tipc_msg_build(struct tipc_msg *mhdr, struct msghdr *m, int offset,
>   	int pktno = 1;
>   	char *pktpos;
>   	int pktsz;
> +	struct iovec iov;
>   	int rc;
>   
>   	msg_set_size(mhdr, msz);
>   
> +	if (!dsz)
> +		iov_iter_init(&m->msg_iter, ITER_SOURCE, &iov, 0, 0);
> +
>   	/* No fragmentation needed? */
>   	if (likely(msz <= pktmax)) {
>   		skb = tipc_buf_acquire(msz, GFP_KERNEL);

I feel a little uncomfortable with adding an uninitialized struct iovec 
here.

Al Viro had a different proposal in his email from Dec 8th:

On 2022-12-11 04:38, Al Viro wrote:
> On Thu, Dec 08, 2022 at 08:38:14PM +0100, Eric Dumazet wrote:
>
>> Exposes an old bug in tipc ?
>>
>> Seems a new check added by Al in :
>>
>> Author: Al Viro <vi...@ze...>
>> Date:   Thu Sep 15 20:11:15 2022 -0400
>>
>>      iov_iter: saner checks for attempt to copy to/from iterator
>>
>>      instead of "don't do it to ITER_PIPE" check for ->data_source being
>>      false on copying from iterator.  Check for !->data_source for
>>      copying to iterator, while we are at it.
>>
>>      Signed-off-by: Al Viro <vi...@ze...>
> Lovely...  zero-length sendmsg with uninitialized ->msg_data...
>
> 	I would probably argue that it's a bug in tipc_connect(),
> fixed by iov_iter_kvec(&m.msg_iter, ITER_SOURCE, NULL, 0, 0);
> in there.  Depends - if that kind of uninitialized msg_iter used
> as zero length source or zero length destination is a frequent pattern,
> might as well make zero-byte copy_...iter() succeed quietly;
> I hope it isn't, but that's definitely something I'd missed
> when doing that series.
>
> 	I'll take a look tomorrow^Win the morning, after I get
> some sleep...
>

Did you consider that one?

///jon

[tipc-discussion] [PATCH v1 1/1] tipc: fix kernel warning when sending SYN message

[Tung N. <tun...@de...>]

When sending a SYN message, this kernel stack trace is observed:

...
[   13.396352] RIP: 0010:_copy_from_iter+0xb4/0x550
...
[   13.398494] Call Trace:
[   13.398630]  <TASK>
[   13.398630]  ? __alloc_skb+0xed/0x1a0
[   13.398630]  tipc_msg_build+0x12c/0x670 [tipc]
[   13.398630]  ? shmem_add_to_page_cache.isra.71+0x151/0x290
[   13.398630]  __tipc_sendmsg+0x2d1/0x710 [tipc]
[   13.398630]  ? tipc_connect+0x1d9/0x230 [tipc]
[   13.398630]  ? __local_bh_enable_ip+0x37/0x80
[   13.398630]  tipc_connect+0x1d9/0x230 [tipc]
[   13.398630]  ? __sys_connect+0x9f/0xd0
[   13.398630]  __sys_connect+0x9f/0xd0
[   13.398630]  ? preempt_count_add+0x4d/0xa0
[   13.398630]  ? fpregs_assert_state_consistent+0x22/0x50
[   13.398630]  __x64_sys_connect+0x16/0x20
[   13.398630]  do_syscall_64+0x42/0x90
[   13.398630]  entry_SYSCALL_64_after_hwframe+0x63/0xcd

It is because commit a41dad905e5a ("iov_iter: saner checks for attempt to copy to/from iterator")
has introduced sanity check for copying from/to iov iterator. Lacking
of copy direction from the iterator viewpoint would lead to kernel
stack trace like above.

This commit fixes this issue by initializing the iov iterator with
the correct copy direction.

Signed-off-by: Tung Nguyen <tun...@de...>
---
 net/tipc/msg.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/tipc/msg.c b/net/tipc/msg.c
index 5c9fd4791c4b..f40cd9032b62 100644
--- a/net/tipc/msg.c
+++ b/net/tipc/msg.c
@@ -377,10 +377,14 @@ int tipc_msg_build(struct tipc_msg *mhdr, struct msghdr *m, int offset,
 	int pktno = 1;
 	char *pktpos;
 	int pktsz;
+	struct iovec iov;
 	int rc;
 
 	msg_set_size(mhdr, msz);
 
+	if (!dsz)
+		iov_iter_init(&m->msg_iter, ITER_SOURCE, &iov, 0, 0);
+
 	/* No fragmentation needed? */
 	if (likely(msz <= pktmax)) {
 		skb = tipc_buf_acquire(msz, GFP_KERNEL);
-- 
2.34.1

Re: [tipc-discussion] A slow message receiver member slowing the group multicasting for all members

[Jon M. <jm...@re...>]

On 2023-01-06 15:51, Asebot Fasil Abebe wrote:
> Hi folks,
>
> I'd like to understand the expected behavior when a member of a group is
> slow to pick up a group multicast/broadcast message. From what I'm
> observing on my testing, it seem to slow down the whole messaging bus.
> Please confirm and advise on a workaround if any.
That is the way it works, and there is no obvious workaround, except 
letting the member leave the group.
The whole concept is based on synchronized reception, although not as 
restrictive as ABCAST.
Some receivers are allowed to get ahead of others, but only to a certain 
limit, typically some hundred messages.
If it goes beyond that the whole bus will slow down to retain synchronism.

You could possibly get around it by creating more groups, or having this 
as differing instance from the rest of the group (falling back to group 
multicast), but it all depends on your needs and your design.

///jon

>
> Thanks,
>
> Asebot
>
> _______________________________________________
> tipc-discussion mailing list
> tip...@li...
> https://lists.sourceforge.net/lists/listinfo/tipc-discussion
>