netdev - Re: [PATCH 2/4] [CORE]: datagram: basic memory accounting functions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20071220.034304.212231150.davem@davemloft.net>
Date:	Thu, 20 Dec 2007 03:43:04 -0800 (PST)
From:	David Miller <davem@...emloft.net>
To:	haoki@...hat.com
Cc:	herbert@...dor.apana.org.au, netdev@...r.kernel.org,
	tyasui@...hat.com, mhiramat@...hat.com,
	satoshi.oshima.fk@...achi.com, billfink@...dspring.com,
	andi@...stfloor.org, johnpol@....mipt.ru,
	shemminger@...ux-foundation.org, yoshfuji@...ux-ipv6.org,
	yumiko.sugita.yf@...achi.com
Subject: Re: [PATCH 2/4] [CORE]: datagram: basic memory accounting functions

From: Hideo AOKI <haoki@...hat.com>
Date: Mon, 17 Dec 2007 21:38:17 -0500

Why do we need seperate stream and datagram accounting functions?

Is it just to facilitate things like the following test?

> +static inline int sk_wmem_schedule(struct sock *sk, int size)
> +{
> +	if (sk->sk_type == SOCK_DGRAM)
> +		return sk_datagram_wmem_schedule(sk, size);
> +	else
> +		return 1;
> +}

If so, this can be greatly improved.

All of these other functions are identical copies of the stream
counterparts, they should all be consolidated.

I still see a lot of special casing, instead of large pieces of common
code.

There should be one core set of functions that handle the memory
accounting, regardless of socket type.  Maybe there is one spot where
something like sk->prot->doing_memory_accounting is tested, but that's
it.

I am still very dissatisfied with these changes.  They are
full of special cases, because they mix generic facilities
(the socket memory accounting) with an unrelated issue
(we only support memory accounting for datagram sockets which
are actually UDP).

Also, the memory accounting is done at different parts in
the socket code paths for stream vs. datagram.  This is why
everything is inconsistent, and, a mess.

What's funny is that I absolutely do not care if these changes are
perfect and pass every possible regression test.  Rather, I'm more
concerned that this thing is designed correctly and will allow us to
have one core set of memory accounting functions regardless of socket
type.  As it is coded now, we have two sets of code paths to
fix, two ways of doing the socket accounting, and therefore twice
as much code to maintain and debug.

The whole thing needs to be consistent and without special cases.

The "protocol supports memory accounting" test can be performed, as
you did in this patch, by simply checking if
sk->sk_prot->memory_allocated is non-NULL.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html