Re: [PATCH net] virtio_net: Do not pull payload in skb->head

From: Eric Dumazet <edumazet@google.com>
To: Guenter Roeck <linux@roeck-us.net>
Cc: Eric Dumazet <eric.dumazet@gmail.com>,
	"David S . Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>, netdev <netdev@vger.kernel.org>,
	Xuan Zhuo <xuanzhuo@linux.alibaba.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	Jason Wang <jasowang@redhat.com>,
	virtualization@lists.linux-foundation.org
Subject: Re: [PATCH net] virtio_net: Do not pull payload in skb->head
Date: Mon, 12 Apr 2021 07:48:25 +0200	[thread overview]
Message-ID: <CANn89iLZyvjE-wUxfJ1FtAqZdD3OroObBdR9-bXR=GGb1ZASOw@mail.gmail.com> (raw)
In-Reply-To: <ad232021-d30a-da25-c1d5-0bd79628b5e1@roeck-us.net>

On Mon, Apr 12, 2021 at 12:07 AM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On 4/11/21 2:43 PM, Eric Dumazet wrote:
> > On Sun, Apr 11, 2021 at 11:32 PM Guenter Roeck <linux@roeck-us.net> wrote:
> >>
> >> On 4/11/21 2:23 PM, Eric Dumazet wrote:
> >>> On Sun, Apr 11, 2021 at 10:37 PM Guenter Roeck <linux@roeck-us.net> wrote:
> >>>>
> >>>> On 4/11/21 8:06 AM, Eric Dumazet wrote:
> >>>>> On Sun, Apr 11, 2021 at 3:43 PM Guenter Roeck <linux@roeck-us.net> wrote:
> >>>>>
> >>>>>> This patch causes a virtio-net interface failure when booting sh4 images
> >>>>>> in qemu. The test case is nothing special: Just try to get an IP address
> >>>>>> using udhcpc. If it fails, udhcpc reports:
> >>>>>>
> >>>>>> udhcpc: started, v1.33.0
> >>>>>> udhcpc: sending discover
> >>>>>> FAIL
> >>>>>>
> >>>>>
> >>>>> Can you investigate where the incoming packet is dropped ?
> >>>>>
> >>>>
> >>>> Unless I am missing something, packets are not dropped. It looks more
> >>>> like udhcpc gets bad indigestion in the receive path and exits immediately.
> >>>> Plus, it doesn't happen all the time; sometimes it receives the discover
> >>>> response and is able to obtain an IP address.
> >>>>
> >>>> Overall this is quite puzzling since udhcpc exits immediately when the problem
> >>>> is seen, no matter which option I give it on the command line; it should not
> >>>> really do that.
> >>>
> >>>
> >>> Could you strace both cases and report differences you can spot ?
> >>>
> >>> strace -o STRACE -f -s 1000 udhcpc
> >>>
> >>
> >> I'll give it a try. It will take a while; I'll need to add strace to my root
> >> file systems first.
> >>
> >> As a quick hack, I added some debugging into the kernel; it looks like
> >> the data part of the dhcp discover response may get lost with your patch
> >> in place.
> >
> > Data is not lost, the payload is whole contained in skb frags, which
> > was expected from my patch.
> >
> > Maybe this sh arch does something wrong in this case.
> >
> > This could be checksuming...
> >
> > Please check
> >
> > nstat -n
> > <run udhcpc>
> > nstat
> >
>
> Another tool to install.
>
> While that builds, output from strace:
>
> # strace -o /tmp/STRACE  -f -s 1000 udhcpc -n -q
> udhcpc: started, v1.33.0
> udhcpc: sending discover
> udhcpc: received SIGTERM
>
> and:
>
> ...
> 136   socket(AF_PACKET, SOCK_DGRAM, htons(ETH_P_IP)) = 5
> 136   bind(5, {sa_family=AF_PACKET, sll_protocol=htons(ETH_P_IP), sll_ifindex=if_nametoindex("eth0"), sll_hatype=ARPHRD_NETROM, sll_pkttype=PACKET_HOST, sll_halen=0}, 20) = 0
> 136   setsockopt(5, SOL_PACKET, PACKET_AUXDATA, [1], 4) = 0
> 136   fcntl64(5, F_SETFD, FD_CLOEXEC)   = 0
> 136   socket(AF_INET, SOCK_RAW, IPPROTO_RAW) = 6
> 136   ioctl(6, SIOCGIFINDEX, {ifr_name="eth0", }) = 0
> 136   ioctl(6, SIOCGIFHWADDR, {ifr_name="eth0", ifr_hwaddr={sa_family=ARPHRD_ETHER, sa_data=52:54:00:12:34:56}}) = 0
> 136   close(6)                          = 0
> 136   clock_gettime64(CLOCK_MONOTONIC, {tv_sec=161, tv_nsec=2180242}) = 0
> 136   write(2, "udhcpc: sending discover\n", 25) = 25
> 136   socket(AF_PACKET, SOCK_DGRAM, htons(ETH_P_IP)) = 6
> 136   bind(6, {sa_family=AF_PACKET, sll_protocol=htons(ETH_P_IP), sll_ifindex=if_nametoindex("eth0"), sll_hatype=ARPHRD_NETROM, sll_pkttype=PACKET_HOST, sll_halen=6, sll_addr=[0xff, 0xff, 0xff, 0xff, 0xff, 0xff]}, 20) = 0
> 136   sendto(6, "E\0\1H\0\0\0\0@\21y\246\0\0\0\0\377\377\377\377\0D\0C\0014\200\256\1\1\6\0\257\321\212P\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0RT\0\0224V\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0c\202Sc5\1\1=\7\1RT\0\0224V9\2\2@7\7\1\3\6\f\17\34*<\fudhcp 1.33.0\377\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0", 328, 0, {sa_family=AF_PACKET, sll_protocol=htons(ETH_P_IP), sll_ifindex=if_nametoindex("eth0"), sll_hatype=ARPHRD_NETROM, sll_pkttype=PACKET_HOST, sll_halen=6, sll_addr=[0xff, 0xff, 0xff, 0xff, 0xff, 0xff]}, 20

This does not make sense really. You cut the thing so it is hard to
give a verdict.

Please post the whole strace output.

Thanks.

>
> Guenter