> 2) The same scenario above can happen more easily in case the MTU of
> the links is set differently or when changing. In that case, as long as
> a large message in the failure link's transmq queue was built and
> fragmented with its link's MTU > the other link's one, the issue will
> happen (there is no need of a link synching in advance).
>
> 3) The link synching procedure also faces with the same issue but since
> the link synching is only started upon receipt of a SYNCH_MSG, dropping
> the message will not result in a state deadlock, but it is not expected
> as design.
>
> The 1) & 3) issues are resolved by the previous commit 81e4dd94b214
This is the same as previous commit. The commit ID might be invalid
after it's merged into upstream.
> ("tipc: optimize link synching mechanism") by generating only a dummy
> SYNCH_MSG (i.e. without data) at the link synching, so the size of a
> FAILOVER_MSG if any then will never exceed the link's MTU.
> /**
> + * tipc_msg_fragment - build a fragment skb list for TIPC message
> + *
> + * @skb: TIPC message skb
> + * @hdr: internal msg header to be put on the top of the fragments
> + * @pktmax: max size of a fragment incl. the header
> + * @frags: returned fragment skb list
> + *
> + * Returns 0 if the fragmentation is successful, otherwise: -EINVAL
> + * or -ENOMEM
> + */
> +int tipc_msg_fragment(struct sk_buff *skb, const struct tipc_msg *hdr,
> + int pktmax, struct sk_buff_head *frags)
> +{
> + int pktno, nof_fragms, dsz, dmax, eat;
> + struct tipc_msg *_hdr;
> + struct sk_buff *_skb;
> + u8 *data;
> +
> + /* Non-linear buffer? */
> + if (skb_linearize(skb))
> + return -ENOMEM;
> +
> + data = (u8 *)skb->data;
> + dsz = msg_size(buf_msg(skb));
> + dmax = pktmax - INT_H_SIZE;
> +
> + if (dsz <= dmax || !dmax)
> + return -EINVAL;
> +
> + nof_fragms = dsz / dmax + 1;
> +
> + for (pktno = 1; pktno <= nof_fragms; pktno++) {
> + if (pktno < nof_fragms)
> + eat = dmax;
> + else
> + eat = dsz % dmax;
> +
> + _skb = tipc_buf_acquire(INT_H_SIZE + eat, GFP_ATOMIC);
> + if (!_skb)
> + goto error;
> +
> + skb_orphan(_skb);
> + __skb_queue_tail(frags, _skb);
> +
> + skb_copy_to_linear_data(_skb, hdr, INT_H_SIZE);
> + skb_copy_to_linear_data_offset(_skb, INT_H_SIZE, data, eat);
> + data += eat;
> +
> + _hdr = buf_msg(_skb);
> + msg_set_fragm_no(_hdr, pktno);
> + msg_set_nof_fragms(_hdr, nof_fragms);
> + msg_set_size(_hdr, INT_H_SIZE + eat);
> + }
> + return 0;
> +
In fact we have similar code in tipc_msg_build() where we also fragment
packet if necessary. In order to eliminate redundant code, I suggest we
should extract the common code into a separate function and then
tipc_msg_build() and tipc_msg_fragment() call it.
_______________________________________________
tipc-discussion mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/tipc-discussion