On 02/06/2017 05:12 PM, David Miller wrote:

From: Marc Kleine-Budde <mkl@pengutronix.de>
Date: Mon, 6 Feb 2017 15:50:48 +0100

quoted

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   |