[ovs-dev] [PATCHv8] netdev-afxdp: add new netdev type for AF_XDP.

[William Tu]

On Mon, May 20, 2019 at 3:38 AM Eelco Chaudron <echaudro at redhat.com> wrote:
>
>
>
> On 17 May 2019, at 14:39, Ilya Maximets wrote:
>
> > Hi.
> > Just a few comments to the issues you're listed.
> >
> > Best regards, Ilya Maximets.
> >
> > On 17.05.2019 13:23, Eelco Chaudron wrote:
> >> Hi William,
> >>
> >> First a list of issues I found during some basic testing...
> >>
> >> - When I restart or stop OVS (using the systemctl interface as found
> >> in RHEL) it does not clean up the BFP program causing the restart to
> >> fail:
> >>
> >>   2019-05-10T09:12:11.384Z|00042|netdev_afxdp|ERR|AF_XDP device eno1
> >> reconfig fails
> >>   2019-05-10T09:12:11.384Z|00043|dpif_netdev|ERR|Failed to set
> >> interface eno1 new configuration
> >>
> >>   I need to manually run "ip link set dev eno1 xdp off" to make it
> >> recover.
> >
> > Userspace datapath requires '--cleanup' option passed to 'ovs-appctl
> > exit'
> > to clean up allocated resources. Otherwise datapath will not be
> > destroyed,
> > i.e. netdev will not be destroyed --> no xdp program unloading.
>
> Maybe we should try to reload/cleanup at startup?

OK, will add it into the make check-afxdp cleanup process.
>
> >>
> >> - When I remove a bridge, I get an emer in the revalidator:
> >>
> >>   2019-05-10T09:40:34.401Z|00045|netdev_afxdp|INFO|remove xdp
> >> program
> >>
> >> 2019-05-10T09:40:34.652Z|00001|util(revalidator49)|EMER|lib/poll-loop.c:111:
> >> assertion !fd != !wevent failed in poll_create_node()
> >>
> >
> > This actually should never happen. Looks like a memory corruption.
> >
> >>   Easy to replicate with this:
> >>
> >>     $ ovs-vsctl add-br ovs_pvp_br0 -- set bridge ovs_pvp_br0
> >> datapath_type=netdev
> >>     $ ovs-vsctl add-port ovs_pvp_br0 eno1 -- set interface eno1
> >> type="afxdp" options:xdpmode=drv
> >>     $ ovs-vsctl del-br ovs_pvp_br0
> >>
> >>
> >> - High pmd usage on the statistics, even with no packets is this
> >> expected?
> >>
> >>   $ ovs-appctl dpif-netdev/pmd-rxq-show
> >>   pmd thread numa_id 0 core_id 1:
> >>     isolated : false
> >>     port: dpdk0             queue-id:  0  pmd
> >> usage:  0 %
> >>     port: eno1              queue-id:  0  pmd
> >> usage: 49 %
> >>
> >>   It goes up slowly and gets stuck at 49%
> >>
> >>
> >> - When doing the PVP testing I noticed that the physical port has
> >> odd/no
> >>   tx statistics:
> >>
> >>   $ ovs-ofctl dump-ports ovs_pvp_br0
> >>   OFPST_PORT reply (xid=0x2): 3 ports
> >>     port LOCAL: rx pkts=0, bytes=0, drop=0, errs=0, frame=0,
> >> over=0, crc=0
> >>              tx pkts=0, bytes=0, drop=0, errs=0, coll=0
> >>     port  eno1: rx pkts=103256197, bytes=6195630508, drop=0,
> >> errs=0, frame=0, over=0, crc=0
> >>              tx pkts=0, bytes=19789272440056, drop=0,
> >> errs=0, coll=0
> >>     port  tapVM: rx pkts=4043, bytes=501278, drop=0, errs=0,
> >> frame=0, over=0, crc=0
> >>              tx pkts=4058, bytes=502504, drop=0, errs=0,
> >> coll=0
> >>
> >>
> >> - Packets larger than 1028 bytes are dropped. Guess this needs to be
> >> fixed, and we need to state that jumbo frames are not supported. Are
> >> you planning on adding this?
> >>
> >>   Currently I can find not mentioning of MTU limitation in the
> >> documentation, or any code to prevent it from being changed above the
> >> supported limit.
> >
> > Actually Jumbo frames are supported, but yes, the packet size
> > is limited by the page size. So, jumbo frames up to ~3.5K should
> > be supported without issues.
> > We'll need to determine the upper limit and reject requested mtu
> > if it's larger.
>
> Currently, none-jumbo frames are not even working, and I think a jumbo
> check should be added as we allocate chunks of 2048.

Right, we can re-allocate the chunk to a larger size.
But for now, I will limit it to 2048 and error out when mtu is larger.

>
> >>
> >>
> >> - ovs-vswitchd is still crashing or stops forwarding packets when
> >> trying to do
> >>   PVP testing with Qemu that has a TAP interface doing XDP and
> >> running packets
> >>   at wire speed to the 10G interface.
> >
> > Actually, there are a lot of places in current version where
> > rings/umems could
> > be corrupted leading to unpredictable memory corruptions/crashes/time
> > wasting
> > trying to allocate exhausted resources.
>
> ACK :)

Yes, I'm working on a newer version.

> > Simple: rdtsc.
> >
> > Actually, right now we need to restrict support to only x86_64,
> > because
> > above rdtsc is in 64bit form and will not work for 32bit cpu.
>
> If this is all, why not copy the rest from DPDK and support all…

I don't know AF_XDP is supported for 32bit or not.
For next version, let's just assume x86_64.

Thanks for the feedbacks
William