Re: [PATCH] public/io/netif.h: add a new extra type for XDP

[Jürgen Groß <jgross@xxxxxxxx>]

On 18.05.20 12:37, Denis Kirjanov wrote:
> On 5/18/20, Jürgen Groß <jgross@xxxxxxxx> wrote:
> > On 18.05.20 11:52, Denis Kirjanov wrote:
> > > On 5/18/20, Jürgen Groß <jgross@xxxxxxxx> wrote:
> > > > On 18.05.20 10:24, Denis Kirjanov wrote:
> > > > > The patch adds a new extra type to be able to diffirentiate
> > > > > between RX responses on xen-netfront side with the adjusted offset
> > > > > required for XDP processing.
> > > > >
> > > > > For Linux the offset value is going to be passed via xenstore.
> > > > Why? I can only see disadvantages by using a different communication
> > > > mechanism.
> > > I see it like other features passed through xenstore and it requires
> > > less changes to
> > > other structures with the same final result.
> > This is okay as long there is no Xenstore interaction required when the
> > interface has been setup completely (i.e. only defining the needed
> > offset for XDP is fine, enabling/disabling XDP at runtime should not be
> > done via Xenstore IMO).
> I've checked netfront-<--->netback interaction and found no problems with it.
> Paul found an issue that the value of the netfront state hasn't been
> re-read (during unbind-bind sequence in dom0) and I've fixed it the
> patch for the guest
I don't say your variant isn't working, but a feature being switchable
dynamically AND needing a ring page synchronization anyway should IMO
be switched via a specific ring page request.

I might have not the complete picture, so in case you have a good reason
to do it via Xenstore, fine, but "I'm seeing no problem with it" is no
good reason for a specific design decision.

> > And please, no guest type special casing. Please replace "Linux" by e.g.
> > "The guest" (with additional tweaking of the following sentence).
> Oh, just sent v2. I'll fix a comment.
Thanks.


Juergen