rte_atomic API compatibility & standard atomics

rte_atomic API compatibility & standard atomics

[Tyler Retzlaff]

Hi folks,

I don't think we discussed it specifically but what is the expectation
in relation to converting to standard atomics and compatibility of the
legacy rte_atomic APIs?

We can't really convert the inline function implementations of the
rte_atomic APIs because doing so would break compatibility. This is
because if the implementation uses standard atomics APIs then we are
required to pass _Atomic types to the generic atomic intrinsics.

We can choose to just leave the rte_atomic API implementations as they
are using the GCC builtins and i'm fine with that, but I do need some
help with what to do with msvc then since it doesn't have those
builtins.

The options seem to be as follows.

1.
Just cast the non-atomic types in the rte_atomic APIs implementation
to _Atomic which may work but i'm pretty sure is undefined behavior since
you can't qualify a non _Atomic type to suddenly be _Atomic.

2.
We could conditionally compile (hide) the legacy rte_atomic APIs when
msvc is in use, this seems not bad since there technically aren't any
Windows/MSVC consumers, but if someone wanted to port an existing
application they would have to adapt the code to avoid use of
rte_atomic.

For now I think the safest option is to go with 2 since it doesn't
impose any compatibility risk and conditional compilation only exists
until we deprecate and remove the old rte_atomic APIs.

Are there any other options i'm missing here?

Thanks

RE: rte_atomic API compatibility & standard atomics

[Morten Brørup]

> From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> Sent: Monday, 27 March 2023 21.39
> 
> Hi folks,
> 
> I don't think we discussed it specifically but what is the expectation
> in relation to converting to standard atomics and compatibility of the
> legacy rte_atomic APIs?
> 
> We can't really convert the inline function implementations of the
> rte_atomic APIs because doing so would break compatibility. This is
> because if the implementation uses standard atomics APIs then we are
> required to pass _Atomic types to the generic atomic intrinsics.
> 
> We can choose to just leave the rte_atomic API implementations as they
> are using the GCC builtins and i'm fine with that, but I do need some
> help with what to do with msvc then since it doesn't have those
> builtins.
> 
> The options seem to be as follows.
> 
> 1.
> Just cast the non-atomic types in the rte_atomic APIs implementation
> to _Atomic which may work but i'm pretty sure is undefined behavior
> since
> you can't qualify a non _Atomic type to suddenly be _Atomic.
> 
> 2.
> We could conditionally compile (hide) the legacy rte_atomic APIs when
> msvc is in use, this seems not bad since there technically aren't any
> Windows/MSVC consumers, but if someone wanted to port an existing
> application they would have to adapt the code to avoid use of
> rte_atomic.
> 
> For now I think the safest option is to go with 2 since it doesn't
> impose any compatibility risk and conditional compilation only exists
> until we deprecate and remove the old rte_atomic APIs.
> 
> Are there any other options i'm missing here?
> 
> Thanks

As a variant of your second option, you could make most of the legacy rte_atomic APIs available to MSVC by changing the atomic counter types from volatile to _Atomic. Then only the atomic cmpset() and exchange() functions are unavailable for the application. E.g. for the 32 bit atomic counter type:

typedef struct {
-	volatile int32_t cnt; /**< An internal counter value. */
+	_Atomic int32_t cnt; /**< An internal counter value. */
} rte_atomic32_t;

Re: rte_atomic API compatibility & standard atomics

[Tyler Retzlaff]

On Mon, Mar 27, 2023 at 10:08:10PM +0200, Morten Brørup wrote:
> > From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> > Sent: Monday, 27 March 2023 21.39
> > 
> > Hi folks,
> > 
> > I don't think we discussed it specifically but what is the expectation
> > in relation to converting to standard atomics and compatibility of the
> > legacy rte_atomic APIs?
> > 
> > We can't really convert the inline function implementations of the
> > rte_atomic APIs because doing so would break compatibility. This is
> > because if the implementation uses standard atomics APIs then we are
> > required to pass _Atomic types to the generic atomic intrinsics.
> > 
> > We can choose to just leave the rte_atomic API implementations as they
> > are using the GCC builtins and i'm fine with that, but I do need some
> > help with what to do with msvc then since it doesn't have those
> > builtins.
> > 
> > The options seem to be as follows.
> > 
> > 1.
> > Just cast the non-atomic types in the rte_atomic APIs implementation
> > to _Atomic which may work but i'm pretty sure is undefined behavior
> > since
> > you can't qualify a non _Atomic type to suddenly be _Atomic.
> > 
> > 2.
> > We could conditionally compile (hide) the legacy rte_atomic APIs when
> > msvc is in use, this seems not bad since there technically aren't any
> > Windows/MSVC consumers, but if someone wanted to port an existing
> > application they would have to adapt the code to avoid use of
> > rte_atomic.
> > 
> > For now I think the safest option is to go with 2 since it doesn't
> > impose any compatibility risk and conditional compilation only exists
> > until we deprecate and remove the old rte_atomic APIs.
> > 
> > Are there any other options i'm missing here?
> > 
> > Thanks
> 
> As a variant of your second option, you could make most of the legacy rte_atomic APIs available to MSVC by changing the atomic counter types from volatile to _Atomic. Then only the atomic cmpset() and exchange() functions are unavailable for the application. E.g. for the 32 bit atomic counter type:
> 
> typedef struct {
> -	volatile int32_t cnt; /**< An internal counter value. */
> +	_Atomic int32_t cnt; /**< An internal counter value. */
> } rte_atomic32_t;
> 

it's a good suggestion. but i'm not sure i want to get bogged down
making an old api available that hopefully we will remove soon.

though i'm still torn because i would really like the path to use msvc
for any application to be lower burden.

unless there are objections i think i'll do 2 as is. if good progress is
made we can re-evaluate doing the extra work to make available the old apis
as you suggest or potentially leave them unavailable forever subject to
any plans to deprecate and remove them.

thanks!

RE: rte_atomic API compatibility & standard atomics

[Morten Brørup]

> From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> Sent: Tuesday, 28 March 2023 20.46
> 
> On Mon, Mar 27, 2023 at 10:08:10PM +0200, Morten Brørup wrote:
> > > From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> > > Sent: Monday, 27 March 2023 21.39
> > >
> > > Hi folks,
> > >
> > > I don't think we discussed it specifically but what is the expectation
> > > in relation to converting to standard atomics and compatibility of the
> > > legacy rte_atomic APIs?
> > >
> > > We can't really convert the inline function implementations of the
> > > rte_atomic APIs because doing so would break compatibility. This is
> > > because if the implementation uses standard atomics APIs then we are
> > > required to pass _Atomic types to the generic atomic intrinsics.
> > >
> > > We can choose to just leave the rte_atomic API implementations as they
> > > are using the GCC builtins and i'm fine with that, but I do need some
> > > help with what to do with msvc then since it doesn't have those
> > > builtins.
> > >
> > > The options seem to be as follows.
> > >
> > > 1.
> > > Just cast the non-atomic types in the rte_atomic APIs implementation
> > > to _Atomic which may work but i'm pretty sure is undefined behavior
> > > since
> > > you can't qualify a non _Atomic type to suddenly be _Atomic.

This could also be an option, wrapped in #ifdef MSVC, so they are still unchanged for other build environments.

That limits your concern about undefined behavior to specifically how MSVC behaves.

> > >
> > > 2.
> > > We could conditionally compile (hide) the legacy rte_atomic APIs when
> > > msvc is in use, this seems not bad since there technically aren't any
> > > Windows/MSVC consumers, but if someone wanted to port an existing
> > > application they would have to adapt the code to avoid use of
> > > rte_atomic.
> > >
> > > For now I think the safest option is to go with 2 since it doesn't
> > > impose any compatibility risk and conditional compilation only exists
> > > until we deprecate and remove the old rte_atomic APIs.
> > >
> > > Are there any other options i'm missing here?
> > >
> > > Thanks
> >
> > As a variant of your second option, you could make most of the legacy
> rte_atomic APIs available to MSVC by changing the atomic counter types from
> volatile to _Atomic. Then only the atomic cmpset() and exchange() functions
> are unavailable for the application. E.g. for the 32 bit atomic counter type:
> >
> > typedef struct {
> > -	volatile int32_t cnt; /**< An internal counter value. */
> > +	_Atomic int32_t cnt; /**< An internal counter value. */
> > } rte_atomic32_t;
> >
> 
> it's a good suggestion. but i'm not sure i want to get bogged down
> making an old api available that hopefully we will remove soon.
> 
> though i'm still torn because i would really like the path to use msvc
> for any application to be lower burden.
> 
> unless there are objections i think i'll do 2 as is. if good progress is
> made we can re-evaluate doing the extra work to make available the old apis
> as you suggest or potentially leave them unavailable forever subject to
> any plans to deprecate and remove them.

No objections from me, either way.

From a high level perspective, I consider it perfectly reasonable to get up and running with very limited support. When MSVC gets more traction, and MSVC users want more of DPDK, I expect to see questions on the mailing list, or directly to you or the MSVC team. Then you can focus catching up on the features in demand.