On 02/06/2017 05:12 PM, David Miller wrote: > From: Marc Kleine-Budde <m...@pengutronix.de> > 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