> 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

Reply via email to