Re: bug: virtio PMD sends malformed packets for 32-bit processes on 64-bit kernel

["Tan, Jianfeng" <jianfeng.tan@intel.com>]

> -----Original Message-----
> From: Frederico Cadete [mailto:Frederico.Cadete-ext@oneaccess-net.com]
> Sent: Tuesday, June 27, 2017 9:32 PM
> To: yuanhan.liu@linux.intel.com; maxime.coquelin@redhat.com; Tan,
> Jianfeng
> Cc: John Sucaet; dev@dpdk.org
> Subject: Re: bug: virtio PMD sends malformed packets for 32-bit processes
> on 64-bit kernel
...
> > >
> > > In that case I guess that the issue is that the conversion is
> > > assuming
> > > that on 32-bit apps only 4 bytes are necessary, even in the case of
> > > virtio-pci and 64-bit physaddr.
> > >
> > > Would you say that this is how vring_desc's addr should be filled?
> > >
> > >              |   32-bit app          | 64-bit app             |
> > > ------------+-----------------------+ -----------------------+
> > > virtio-pci  | buf_physaddr, 8 bytes | buf_physaddr, 8 bytes  |
> > > virtio-user | buf_addr, 4 bytes     | buf_addr, 8 bytes      |
> > >
> > > I believe that the issue is that after commit 260aae9a, for virtio-
> > > pci
> > > and 32-bit app it is taking 4 bytes instead of 8.
> > Aha, yes, that's the issue! Great analysis. After Bruce's commit
> > 586ec205bcbbb ("mbuf: fix 64-bit address alignment in 32-bit
> > builds"),
> > we can fix this issue by fetching 8 bytes at all cases. But
> > unfortunately, that commit is not back-ported to v17.02.1.
> 
> I don't see how changing the alignment of buf_physaddr allows fetching
> 8 bytes in all cases, even in the case of 32-bit virtio-user where what
> we need are 4 bytes from buf_addr. Am I missing something?

After that, | 4-byte buf_addr | 4-byte padding | 8-byte buf_physaddr| for 32-bit system, right?

I would expect 4-byte padding are all-zero. So we can just fetch all first 8-byte outside.


> 
> Besides, Bruce's patch changes the memory layout of rte_mbuf. A priori
> that's not the kind I would like to find in an update of a stable
> branch :)

Yes, you are right. Stable branches won't accept such change.

> 
> >
> > I wonder if we can back-port Bruce's patch with a new patch to fix
> > this
> > problem?
> >
> > Any opinions from others?
> >
> > Thanks,
> > Jianfeng
> >
> > >
> > >
> > > >
> > > > >
> > > > > Any suggestions on if and how this could be fixed?
> > > > >
> > > > > Meanwhile, the bug affects dpdk 17.05, 17.02.1 and master.
> > > > > Users
> > > > > not
> > > > > requiring virtio-user support can avoid it by setting
> > > > > CONFIG_VIRTIO_USER=n during compilation.
> > > > >
> > > > > Best regards,
> > > > > Frederico Cadete