Re: [linux-nfc] [PATCH 2/2] NFC: Correct outgoing frame before requeueing

[Eric Lapuyade <eric.lapuyade@linux.intel.com>]

On 09/06/2012 12:22 PM, Waldemar Rymarkiewicz wrote:
> Purge data added by lower layers (len and crc) in the head and tail of the frame
> during initial send. Now, the frame is correct to be resent.
> 
> Signed-off-by: Waldemar Rymarkiewicz <waldemar.rymarkiewicz@tieto.com>
> ---
>  net/nfc/hci/shdlc.c |    5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/net/nfc/hci/shdlc.c b/net/nfc/hci/shdlc.c
> index 8a5f034..30e353e 100644
> --- a/net/nfc/hci/shdlc.c
> +++ b/net/nfc/hci/shdlc.c
> @@ -241,8 +241,9 @@ static void nfc_shdlc_requeue_ack_pending(struct nfc_shdlc *shdlc)
>  	pr_debug("ns reset to %d\n", shdlc->dnr);
>  
>  	while ((skb = skb_dequeue_tail(&shdlc->ack_pending_q))) {
> -		skb_pull(skb, 2);	/* remove len+control */
> -		skb_trim(skb, skb->len - 2);	/* remove crc */
> +		/* remove client head + shdlc control field */
> +		skb_pull(skb, shdlc->client_headroom + 1);
> +		skb_trim(skb, skb->len - shdlc->client_tailroom);
>  		skb_queue_head(&shdlc->send_q, skb);
>  	}
>  	shdlc->ns = shdlc->dnr;
> 

Hi Waldemar,

This patch would work for PN544, but it makes the assumption that the
driver will always insert/append exactly client_headroom/client_tailroom
bytes when xmit is called. This is not specified nor enforced so it may
be a little dangerous.

To correct that, we can :
- either specify that the driver xmit MUST NOT modify skb. We then let
it remove those len and crc before returning from xmit. This is the most
logical since only the driver really knows what it's doing.
- or specify that the driver head/tailroom request MUST BE the exact
number of bytes added to the skb by xmit.

Samuel, what do you think?

Eric