netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Marc Kleine-Budde <mkl@pengutronix.de>
To: David Miller <davem@davemloft.net>
Cc: netdev@vger.kernel.org, kernel@pengutronix.de, linux-can@vger.kernel.org
Subject: Re: pull-request: can-next 2017-02-06,pull-request: can-next 2017-02-06
Date: Mon, 6 Feb 2017 17:43:43 +0100	[thread overview]
Message-ID: <809cdb71-a29c-f952-0cab-ff643b4906f0@pengutronix.de> (raw)
In-Reply-To: <20170206.111259.2261971436931731059.davem@davemloft.net>


[-- Attachment #1.1: Type: text/plain, Size: 2065 bytes --]

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
> 
>> 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   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

  reply	other threads:[~2017-02-06 16:43 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-06 14:50 pull-request: can-next 2017-02-06 Marc Kleine-Budde
2017-02-06 16:12 ` pull-request: can-next 2017-02-06,pull-request: " David Miller
2017-02-06 16:43   ` Marc Kleine-Budde [this message]
2017-02-06 16:59     ` pull-request: can-next 2017-02-06,pull-request: can-next 2017-02-06,Re: " David Miller
2017-02-09  9:52       ` Marc Kleine-Budde

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=809cdb71-a29c-f952-0cab-ff643b4906f0@pengutronix.de \
    --to=mkl@pengutronix.de \
    --cc=davem@davemloft.net \
    --cc=kernel@pengutronix.de \
    --cc=linux-can@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).