[MPTCP] Re: [RFC PATCH 2/6] Squash-to: "mptcp: Add path manager interface"

[MPTCP] Re: [RFC PATCH 2/6] Squash-to: "mptcp: Add path manager interface"

[Florian Westphal]

[-- Attachment #1: Type: text/plain, Size: 475 bytes --]

Paolo Abeni <pabeni(a)redhat.com> wrote:
> +struct mptcp_pm_data {
> +	struct mptcp_addr_info local;
> +	struct mptcp_addr_info remote;
> +
> +	spinlock_t	lock;
> +
> +	bool		addr_signal;
> +	bool		server_side;
> +	bool		fully_established;
> +	enum mptcp_pm_status status;
> +	u8		add_addr_signaled;
> +	u8		add_addr_accepted;

Why are the first three bool and the rest u8?

We should probably avoid "bool" in structs.
Perhaps use foo:1 as most of net stack?

[MPTCP] Re: [RFC PATCH 2/6] Squash-to: "mptcp: Add path manager interface"

[Paolo Abeni]

[-- Attachment #1: Type: text/plain, Size: 1000 bytes --]

On Wed, 2020-02-12 at 15:22 +0100, Florian Westphal wrote:
> Paolo Abeni <pabeni(a)redhat.com> wrote:
> > +struct mptcp_pm_data {
> > +	struct mptcp_addr_info local;
> > +	struct mptcp_addr_info remote;
> > +
> > +	spinlock_t	lock;
> > +
> > +	bool		addr_signal;
> > +	bool		server_side;
> > +	bool		fully_established;
> > +	enum mptcp_pm_status status;
> > +	u8		add_addr_signaled;
> > +	u8		add_addr_accepted;
> 
> Why are the first three bool and the rest u8?

'addr_signal', 'server_side' and 'fully_established' have bool
semantics, while the others are counter, maxing at 255.

> We should probably avoid "bool" in structs.

IIRC the bitfield usage force the compiler to emit worse code, as such
I thought it was suggested only for size-critical structs?!? (mptcp
pernet does not have stict memory constraint).

> Perhaps use foo:1 as most of net stack?

uhm?!? quick && dirty grep count shows more bool field than ':1;' under
include/net/ !?!

Thanks,

Paolo