| Message ID | 1426006607.11398.5.camel@edumazet-glaptop2.roam.corp.google.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | stephen hemminger |
| Headers | show |
On Tue, 2015-03-10 at 09:56 -0700, Eric Dumazet wrote: > From: Eric Dumazet <edumazet@google.com> > > Socket cookies are 64bit, even if ss happens to be > a 32bit binary, running on a 64 bit host. > > Signed-off-by: Eric Dumazet <edumazet@google.com> > --- > misc/ss.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/misc/ss.c b/misc/ss.c > index 21a4366..ba58894 100644 > --- a/misc/ss.c > +++ b/misc/ss.c > @@ -679,9 +679,9 @@ static inline char *sock_addr_get_str(const inet_prefix *prefix) > return tmp; > } > > -static unsigned long cookie_sk_get(uint32_t *cookie) > +static unsigned long long cookie_sk_get(const uint32_t *cookie) > { > - return (((unsigned long)cookie[1] << 31) << 1) | cookie[0]; > + return (((unsigned long long)cookie[1] << 31) << 1) | cookie[0]; Then you should also use << 32 instead of splitting it into two shifts. Ben. > } > > static const char *sstate_name[] = {
On Sat, 2015-03-14 at 19:19 +0000, Ben Hutchings wrote: > On Tue, 2015-03-10 at 09:56 -0700, Eric Dumazet wrote: > > From: Eric Dumazet <edumazet@google.com> > > -static unsigned long cookie_sk_get(uint32_t *cookie) > > +static unsigned long long cookie_sk_get(const uint32_t *cookie) > > { > > - return (((unsigned long)cookie[1] << 31) << 1) | cookie[0]; > > + return (((unsigned long long)cookie[1] << 31) << 1) | cookie[0]; > > Then you should also use << 32 instead of splitting it into two shifts. Yep, why not, although compiler does not care : no shift at all is generated anyway. Since kernel code no longer uses this << 31 trick, we can align ss code. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Ben Hutchings > On Tue, 2015-03-10 at 09:56 -0700, Eric Dumazet wrote: > > From: Eric Dumazet <edumazet@google.com> > > > > Socket cookies are 64bit, even if ss happens to be > > a 32bit binary, running on a 64 bit host. > > > > Signed-off-by: Eric Dumazet <edumazet@google.com> > > --- > > misc/ss.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/misc/ss.c b/misc/ss.c > > index 21a4366..ba58894 100644 > > --- a/misc/ss.c > > +++ b/misc/ss.c > > @@ -679,9 +679,9 @@ static inline char *sock_addr_get_str(const inet_prefix *prefix) > > return tmp; > > } > > > > -static unsigned long cookie_sk_get(uint32_t *cookie) > > +static unsigned long long cookie_sk_get(const uint32_t *cookie) > > { > > - return (((unsigned long)cookie[1] << 31) << 1) | cookie[0]; > > + return (((unsigned long long)cookie[1] << 31) << 1) | cookie[0]; > > Then you should also use << 32 instead of splitting it into two shifts. I wondered if the code should be reading the value in the host's natural endianness? Then the code might be optimisable to: return *(unsigned long long *)cookie; which rather begs the question as to why cookie is uint32_t[2] instead of uint64_t. David
On Mon, 2015-03-16 at 16:39 +0000, David Laight wrote: > I wondered if the code should be reading the value in the host's natural > endianness? > Then the code might be optimisable to: > return *(unsigned long long *)cookie; This will trap on arches requesting 8 bytes alignment. Not all linux hosts run x86 Look at rta_getattr_u64(), and you'll see that manipulating 64bit values in netlink requires a memcpy() because we have no 64bit alignment guarantee. > which rather begs the question as to why cookie is uint32_t[2] instead of > uint64_t. > This question comes too late, inet_diag was added years ago, and we must keep ABI compatibility forever. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/misc/ss.c b/misc/ss.c index 21a4366..ba58894 100644 --- a/misc/ss.c +++ b/misc/ss.c @@ -679,9 +679,9 @@ static inline char *sock_addr_get_str(const inet_prefix *prefix) return tmp; } -static unsigned long cookie_sk_get(uint32_t *cookie) +static unsigned long long cookie_sk_get(const uint32_t *cookie) { - return (((unsigned long)cookie[1] << 31) << 1) | cookie[0]; + return (((unsigned long long)cookie[1] << 31) << 1) | cookie[0]; } static const char *sstate_name[] = {