On 02/06/2017 05:12 PM, David Miller wrote: > From: Marc Kleine-Budde <[email protected]> > Date: Mon, 6 Feb 2017 15:50:48 +0100 > >> this is a pull request of 16 patches for net-next/master. >> >> The first two patches by David Jander and me add the rx-offload >> framework for CAN devices to the kernel. The remaining 14 patches >> convert the flexcan driver to make use of it. > > Pulled, but I wonder if your comparisons does the right thing when the > counters overflow. > > I think you need to do the same thing we do for TCP sequence number > comparisons and code it like: > > static inline bool before(__u32 seq1, __u32 seq2) > { > return (__s32)(seq1-seq2) < 0; > }
Yes, I think it's basically the same as the TCP sequence number code,
but obviously less readable.
> static int can_rx_offload_compare(struct sk_buff *a, struct sk_buff *b)
> {
> const struct can_rx_offload_cb *cb_a, *cb_b;
>
> cb_a = can_rx_offload_get_cb(a);
> cb_b = can_rx_offload_get_cb(b);
>
> /* Substract two u32 and return result as int, to keep
> * difference steady around the u32 overflow.
> */
> return cb_b->timestamp - cb_a->timestamp;
> }
This does the "(__s32)(seq1-seq2)"
> skb_queue_reverse_walk(head, pos) {
> const struct can_rx_offload_cb *cb_pos, *cb_new;
>
> cb_pos = can_rx_offload_get_cb(pos);
> cb_new = can_rx_offload_get_cb(new);
>
> netdev_dbg(new->dev,
> "%s: pos=0x%08x, new=0x%08x, diff=%10d,
> queue_len=%d\n",
> __func__,
> cb_pos->timestamp, cb_new->timestamp,
> cb_new->timestamp - cb_pos->timestamp,
> skb_queue_len(head));
>
> if (compare(pos, new) < 0)
And here the "return ... < 0;"
> continue;
> insert = pos;
> break;
> }
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
signature.asc
Description: OpenPGP digital signature
