All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Loftus, Ciara" <ciara.loftus@intel.com>
To: Stephen Hemminger <stephen@networkplumber.org>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
	"stable@dpdk.org" <stable@dpdk.org>,
	"Yigit, Ferruh" <ferruh.yigit@intel.com>,
	"Zhang, Qi Z" <qi.z.zhang@intel.com>,
	"Burakov, Anatoly" <anatoly.burakov@intel.com>
Subject: Re: [dpdk-dev] [PATCH] net/af_xdp: fix support of secondary process
Date: Thu, 23 Sep 2021 12:33:51 +0000	[thread overview]
Message-ID: <PH0PR11MB4791718F21F80BCABF089A6A8EA39@PH0PR11MB4791.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20210921104524.6cda31b9@hermes.local>

> > > > >
> > > > > Doing basic operations like info_get or get_stats was broken
> > > > > in af_xdp PMD. The info_get would crash because dev->device
> > > > > was NULL in secondary process. Fix this by doing same initialization
> > > > > as af_packet and tap devices.
> > > > >
> > > > > The get_stats would crash because the XDP socket is not open in
> > > > > primary process. As a workaround don't query kernel for dropped
> > > > > packets when called from secondary process.
> > > > >
> > > > > Note: this does not address the other bug which is that transmitting
> > > > > in secondary process is broken because the send() in tx_kick
> > > > > will fail because XDP socket fd is not valid in secondary process.
> > > >
> > > > Hi Stephen,
> > > >
> > > > Apologies for the delayed reply, I was on vacation.
> > > >
> > > > In the Bugzilla report you suggest we:
> > > > "mark AF_XDP as broken in with primary/secondary
> > > > and return an error in probe in secondary process".
> > > > I agree with this suggestion. However with this patch we still permit
> > > secondary, and just make sure it doesn't crash for get_stats. Did you
> change
> > > your mind?
> > > > Personally, I would prefer to have primary/secondary either working
> 100%
> > > or else not allowed at all by throwing an error during probe. What do you
> > > think? Do you have a reason/use case to permit secondary processes
> despite
> > > some features not being available eg. full stats, tx?
> > > >
> > > > Thanks,
> > > > Ciara
> > >
> > > There are two cases where secondary is useful even if send/receive can't
> > > work from secondary process.
> > > The pdump and proc-info applications can work with these patches.
> > >
> > > I am using XDP over pdump as an easy way to get packets into the code
> for
> > > testing.
> > >
> > > The flag in the documentation doesn't have a "limited" version.
> > > If you want, will send another patch to disable secondary support.
> >
> > Thanks for explaining. Since there are use cases for secondary, even if the
> functionality is limited, I don't think it should be disabled.
> > Since we can't flag it as 'limited' in the feature matrix, could you please add
> a note about the send/receive limitation in the AF_XDP PMD documentation
> in a v2? There are already a number of limitations listed, which you can add
> to.
> >
> > Thanks,
> > Ciara
> >
> > >
> > > Supporting secondary, means adding a mechanism to pass the socket
> > > around.
> 
> Looking at this in more detail, and my recommendation is:
> For 21.11 release (and mark it as Fixes so it gets backported). Have the driver
> return -ENOTSUP in secondary process.
> 
> For 22.03 add real secondary support using the rte_mp_msg to pass
> necessary
> state to secondary process. Includes socket (and other memory regions?).
> 
> The pdump and proc-info cases are only useful for developer testing, and
> there are
> other ways to do the same thing.


+1 that sounds reasonable to me.

Thanks,
Ciara

  reply	other threads:[~2021-09-23 12:34 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-03 16:15 [dpdk-dev] [PATCH] net/af_xdp: fix support of secondary process Stephen Hemminger
2021-09-15  8:19 ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit
2021-09-20 13:23 ` [dpdk-dev] " Loftus, Ciara
2021-09-20 14:43   ` Stephen Hemminger
2021-09-20 15:09     ` Loftus, Ciara
2021-09-21 17:45       ` Stephen Hemminger
2021-09-23 12:33         ` Loftus, Ciara [this message]
2021-10-05 15:02           ` Ferruh Yigit

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=PH0PR11MB4791718F21F80BCABF089A6A8EA39@PH0PR11MB4791.namprd11.prod.outlook.com \
    --to=ciara.loftus@intel.com \
    --cc=anatoly.burakov@intel.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=qi.z.zhang@intel.com \
    --cc=stable@dpdk.org \
    --cc=stephen@networkplumber.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.