[PATCH 11/11] pulsecore: srbchannel: Enable memfd support; pump protocol version

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 2016-01-13 at 00:19 +0200, Ahmed S. Darwish wrote:
> Hi Tanu,
> 
> On Thu, Oct 01, 2015 at 11:02:55AM +0300, Tanu Kaskinen wrote:
> > On Sun, 2015-09-20 at 23:39 +0200, Ahmed S. Darwish wrote:
> > > diff --git a/src/pulse/context.c b/src/pulse/context.c
> > > index ede01fa..f272cd6 100644
> > > --- a/src/pulse/context.c
> > > +++ b/src/pulse/context.c
> > > @@ -591,6 +591,8 @@ static void setup_context(pa_context *c, pa_iochannel *io) {
> > >      pa_tagstruct_putu32(t, PA_PROTOCOL_VERSION | (c->do_shm ? 0x80000000U : 0));
> > >      pa_tagstruct_put_arbitrary(t, cookie, sizeof(cookie));
> > >  
> > > +    pa_tagstruct_put_boolean(t, pa_memfd_is_supported());
> > 
> > This breaks compatibility with old servers that expect the tagstruct to
> > end after the cookie.
> > 
> > The "memfd supported" bit could be added to the version field like the
> > "shm supported" bit.
> > 
> 
> Unfortunately this will also break compatibility with old servers.
> 
> Right now, PA uses the client's version number most-significant bit as
> a marker for SHM support.
> 
> Afterwards, the server clears _only_ that bit:
> 
>     if (c->version >= 13) {
>         shm_on_remote = !!(c->version & 0x80000000U);
>         c->version &= 0x7FFFFFFFU;
>     }
> 
> So if I overload any of the bits above, e.g. the second MSB bit
> 0x40000000, "old" servers will not clear it and will interpret the
> client version as 0x4000001F instead of just v31 0x1F :-(

That should be ok, since 0x4000001F will be larger than the old
server's version, so the negotiated version will be the old server's
version. The same thing happens when you have version 12 server and
version 13 client with shm support. The server doesn't clear the MSB,
so it thinks the client has very large version.

> > Another approach would be to do add a new command
> > to do memfd negotiation before enabling srbchannel. Abusing the version
> > field is ugly, adding another negotiation adds more connection setup
> > overhead. I don't know which approach we should choose.
> > 
> 
> Can we agree that _from this point forward_, the server clears the
> client's version most-significant _16_ bits? This will give us 15
> free new flags to play with.

I can agree to that. 16 bits should still be plenty for the version
number.

-- 
Tanu


[Index of Archives]     [Linux Audio Users]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux