All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Franklin S Cooper Jr." <fcooper@ti.com>
To: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Cc: Sam Kumar <samanthakumar@google.com>,
	Willem de Bruijn <willemb@google.com>,
	Eric Dumazet <edumazet@google.com>,
	tony@atomide.com, mugunthanvnm@ti.com,
	Network Development <netdev@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	David Miller <davem@davemloft.net>,
	kuznet@ms2.inr.ac.ru, jmorris@namei.org, yoshfuji@linux-ipv6.org,
	kaber@trash.net, nsekhar@ti.com, linux-omap@vger.kernel.org
Subject: Re: Boot failure when using NFS on OMAP based evms
Date: Thu, 7 Apr 2016 09:36:57 -0500	[thread overview]
Message-ID: <57067089.4080406@ti.com> (raw)
In-Reply-To: <CAF=yD-L=HZtD+LaKt1y4OcY0CQKOk_jmXZcQcyXjdfrPLpAhEQ@mail.gmail.com>



On 04/06/2016 09:22 PM, Willem de Bruijn wrote:
> On Wed, Apr 6, 2016 at 7:12 PM, Franklin S Cooper Jr. <fcooper@ti.com> wrote:
>> Hi All,
>>
>> Currently linux-next is failing to boot via NFS on my AM335x GP evm,
>> AM437x GP evm and Beagle X15. I bisected the problem down to the commit
>> "udp: remove headers from UDP packets before queueing".
>>
>> I had to revert the following three commits to get things working again:
>>
>> e6afc8ace6dd5cef5e812f26c72579da8806f5ac
>> udp: remove headers from UDP packets before queueing
>>
>> 627d2d6b550094d88f9e518e15967e7bf906ebbf
>> udp: enable MSG_PEEK at non-zero offset
>>
>> b9bb53f3836f4eb2bdeb3447be11042bd29c2408
>> sock: convert sk_peek_offset functions to WRITE_ONCE
>>
> 
> Thanks for the report, and apologies for breaking your configuration.
> I had missed that sunrpc can dequeue skbs from a udp receive
> queue and makes assumptions about the layout of those packets. rxrpc
> does the same. From what I can tell so far, those are the only two
> protocols that do this. I have verified that the following fixes rxrpc for me
> 
> --- a/net/rxrpc/ar-input.c
> +++ b/net/rxrpc/ar-input.c
> @@ -612,9 +612,9 @@ int rxrpc_extract_header(struct rxrpc_skb_priv
> *sp, struct sk_buff *skb)
>         struct rxrpc_wire_header whdr;
> 
>         /* dig out the RxRPC connection details */
> -       if (skb_copy_bits(skb, sizeof(struct udphdr), &whdr, sizeof(whdr)) < 0)
> +       if (skb_copy_bits(skb, 0, &whdr, sizeof(whdr)) < 0)
>                 return -EBADMSG;
> -       if (!pskb_pull(skb, sizeof(struct udphdr) + sizeof(whdr)))
> +       if (!pskb_pull(skb, sizeof(whdr)))
>                 BUG();
> 
> I have not yet been able to reproduce the sunrpc/nfs issue, but I
> suspect that the following might fix it. I will try to create an NFS
> setup.
> 
> diff --git a/net/sunrpc/socklib.c b/net/sunrpc/socklib.c
> index 2df87f7..8ab40ba 100644
> --- a/net/sunrpc/socklib.c
> +++ b/net/sunrpc/socklib.c
> @@ -155,7 +155,7 @@ int csum_partial_copy_to_xdr(struct xdr_buf *xdr,
> struct sk_buff *skb)
>         struct xdr_skb_reader   desc;
> 
>         desc.skb = skb;
> -       desc.offset = sizeof(struct udphdr);
> +       desc.offset = 0;
>         desc.count = skb->len - desc.offset;
> 
>         if (skb_csum_unnecessary(skb))
> diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
> index 1413cdc..71d6072 100644
> --- a/net/sunrpc/svcsock.c
> +++ b/net/sunrpc/svcsock.c
> @@ -617,7 +617,7 @@ static int svc_udp_recvfrom(struct svc_rqst *rqstp)
>         svsk->sk_sk->sk_stamp = skb->tstamp;
>         set_bit(XPT_DATA, &svsk->sk_xprt.xpt_flags); /* there may be
> more data... */
> 
> -       len  = skb->len - sizeof(struct udphdr);
> +       len  = skb->len;
>         rqstp->rq_arg.len = len;
> 
>         rqstp->rq_prot = IPPROTO_UDP;
> @@ -641,8 +641,7 @@ static int svc_udp_recvfrom(struct svc_rqst *rqstp)
>                 skb_free_datagram_locked(svsk->sk_sk, skb);
>         } else {
>                 /* we can use it in-place */
> -               rqstp->rq_arg.head[0].iov_base = skb->data +
> -                       sizeof(struct udphdr);
> +               rqstp->rq_arg.head[0].iov_base = skb->data;
>                 rqstp->rq_arg.head[0].iov_len = len;
>                 if (skb_checksum_complete(skb))
>                         goto out_free;
> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
> index 65e7595..c1fc7b2 100644
> --- a/net/sunrpc/xprtsock.c
> +++ b/net/sunrpc/xprtsock.c
> @@ -995,15 +995,14 @@ static void xs_udp_data_read_skb(struct rpc_xprt *xprt,
>         u32 _xid;
>         __be32 *xp;
> 
> -       repsize = skb->len - sizeof(struct udphdr);
> +       repsize = skb->len;
>         if (repsize < 4) {
>                 dprintk("RPC:       impossible RPC reply size %d!\n", repsize);
>                 return;
>         }
> 
> 
> 
>         /* Copy the XID from the skb... */
> -       xp = skb_header_pointer(skb, sizeof(struct udphdr),
> -                               sizeof(_xid), &_xid);
> +       xp = skb_header_pointer(skb, 0, sizeof(_xid), &_xid);
>         if (xp == NULL)
>                 return;
> 


Thank you for your quick response. I verified with all of the above
suggested changes that NFS works again on my 3 evms.

WARNING: multiple messages have this Message-ID (diff)
From: "Franklin S Cooper Jr." <fcooper@ti.com>
To: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Cc: Sam Kumar <samanthakumar@google.com>,
	Willem de Bruijn <willemb@google.com>,
	Eric Dumazet <edumazet@google.com>, <tony@atomide.com>,
	<mugunthanvnm@ti.com>,
	Network Development <netdev@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	David Miller <davem@davemloft.net>, <kuznet@ms2.inr.ac.ru>,
	<jmorris@namei.org>, <yoshfuji@linux-ipv6.org>, <kaber@trash.net>,
	<nsekhar@ti.com>, <linux-omap@vger.kernel.org>
Subject: Re: Boot failure when using NFS on OMAP based evms
Date: Thu, 7 Apr 2016 09:36:57 -0500	[thread overview]
Message-ID: <57067089.4080406@ti.com> (raw)
In-Reply-To: <CAF=yD-L=HZtD+LaKt1y4OcY0CQKOk_jmXZcQcyXjdfrPLpAhEQ@mail.gmail.com>



On 04/06/2016 09:22 PM, Willem de Bruijn wrote:
> On Wed, Apr 6, 2016 at 7:12 PM, Franklin S Cooper Jr. <fcooper@ti.com> wrote:
>> Hi All,
>>
>> Currently linux-next is failing to boot via NFS on my AM335x GP evm,
>> AM437x GP evm and Beagle X15. I bisected the problem down to the commit
>> "udp: remove headers from UDP packets before queueing".
>>
>> I had to revert the following three commits to get things working again:
>>
>> e6afc8ace6dd5cef5e812f26c72579da8806f5ac
>> udp: remove headers from UDP packets before queueing
>>
>> 627d2d6b550094d88f9e518e15967e7bf906ebbf
>> udp: enable MSG_PEEK at non-zero offset
>>
>> b9bb53f3836f4eb2bdeb3447be11042bd29c2408
>> sock: convert sk_peek_offset functions to WRITE_ONCE
>>
> 
> Thanks for the report, and apologies for breaking your configuration.
> I had missed that sunrpc can dequeue skbs from a udp receive
> queue and makes assumptions about the layout of those packets. rxrpc
> does the same. From what I can tell so far, those are the only two
> protocols that do this. I have verified that the following fixes rxrpc for me
> 
> --- a/net/rxrpc/ar-input.c
> +++ b/net/rxrpc/ar-input.c
> @@ -612,9 +612,9 @@ int rxrpc_extract_header(struct rxrpc_skb_priv
> *sp, struct sk_buff *skb)
>         struct rxrpc_wire_header whdr;
> 
>         /* dig out the RxRPC connection details */
> -       if (skb_copy_bits(skb, sizeof(struct udphdr), &whdr, sizeof(whdr)) < 0)
> +       if (skb_copy_bits(skb, 0, &whdr, sizeof(whdr)) < 0)
>                 return -EBADMSG;
> -       if (!pskb_pull(skb, sizeof(struct udphdr) + sizeof(whdr)))
> +       if (!pskb_pull(skb, sizeof(whdr)))
>                 BUG();
> 
> I have not yet been able to reproduce the sunrpc/nfs issue, but I
> suspect that the following might fix it. I will try to create an NFS
> setup.
> 
> diff --git a/net/sunrpc/socklib.c b/net/sunrpc/socklib.c
> index 2df87f7..8ab40ba 100644
> --- a/net/sunrpc/socklib.c
> +++ b/net/sunrpc/socklib.c
> @@ -155,7 +155,7 @@ int csum_partial_copy_to_xdr(struct xdr_buf *xdr,
> struct sk_buff *skb)
>         struct xdr_skb_reader   desc;
> 
>         desc.skb = skb;
> -       desc.offset = sizeof(struct udphdr);
> +       desc.offset = 0;
>         desc.count = skb->len - desc.offset;
> 
>         if (skb_csum_unnecessary(skb))
> diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
> index 1413cdc..71d6072 100644
> --- a/net/sunrpc/svcsock.c
> +++ b/net/sunrpc/svcsock.c
> @@ -617,7 +617,7 @@ static int svc_udp_recvfrom(struct svc_rqst *rqstp)
>         svsk->sk_sk->sk_stamp = skb->tstamp;
>         set_bit(XPT_DATA, &svsk->sk_xprt.xpt_flags); /* there may be
> more data... */
> 
> -       len  = skb->len - sizeof(struct udphdr);
> +       len  = skb->len;
>         rqstp->rq_arg.len = len;
> 
>         rqstp->rq_prot = IPPROTO_UDP;
> @@ -641,8 +641,7 @@ static int svc_udp_recvfrom(struct svc_rqst *rqstp)
>                 skb_free_datagram_locked(svsk->sk_sk, skb);
>         } else {
>                 /* we can use it in-place */
> -               rqstp->rq_arg.head[0].iov_base = skb->data +
> -                       sizeof(struct udphdr);
> +               rqstp->rq_arg.head[0].iov_base = skb->data;
>                 rqstp->rq_arg.head[0].iov_len = len;
>                 if (skb_checksum_complete(skb))
>                         goto out_free;
> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
> index 65e7595..c1fc7b2 100644
> --- a/net/sunrpc/xprtsock.c
> +++ b/net/sunrpc/xprtsock.c
> @@ -995,15 +995,14 @@ static void xs_udp_data_read_skb(struct rpc_xprt *xprt,
>         u32 _xid;
>         __be32 *xp;
> 
> -       repsize = skb->len - sizeof(struct udphdr);
> +       repsize = skb->len;
>         if (repsize < 4) {
>                 dprintk("RPC:       impossible RPC reply size %d!\n", repsize);
>                 return;
>         }
> 
> 
> 
>         /* Copy the XID from the skb... */
> -       xp = skb_header_pointer(skb, sizeof(struct udphdr),
> -                               sizeof(_xid), &_xid);
> +       xp = skb_header_pointer(skb, 0, sizeof(_xid), &_xid);
>         if (xp == NULL)
>                 return;
> 


Thank you for your quick response. I verified with all of the above
suggested changes that NFS works again on my 3 evms.

  reply	other threads:[~2016-04-07 14:36 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-06 23:12 Boot failure when using NFS on OMAP based evms Franklin S Cooper Jr.
2016-04-06 23:12 ` Franklin S Cooper Jr.
2016-04-07  2:22 ` Willem de Bruijn
2016-04-07 14:36   ` Franklin S Cooper Jr. [this message]
2016-04-07 14:36     ` Franklin S Cooper Jr.
2016-04-07 14:39     ` Willem de Bruijn
2016-04-07 15:52       ` Willem de Bruijn

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=57067089.4080406@ti.com \
    --to=fcooper@ti.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=jmorris@namei.org \
    --cc=kaber@trash.net \
    --cc=kuznet@ms2.inr.ac.ru \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=mugunthanvnm@ti.com \
    --cc=netdev@vger.kernel.org \
    --cc=nsekhar@ti.com \
    --cc=samanthakumar@google.com \
    --cc=tony@atomide.com \
    --cc=willemb@google.com \
    --cc=willemdebruijn.kernel@gmail.com \
    --cc=yoshfuji@linux-ipv6.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.