ethtool - manual changes in ethtool-copy.h

ethtool - manual changes in ethtool-copy.h

[Michal Kubecek]

Hello,

I'm sorry I didn't notice earlier but ethtool commit 4df55c81996d
("ethtool: change to new sane powerpc64 kernel headers") adds changes to
ethtool-copy.h which are not in sync with kernel file it is generated
from.

This file is supposed to be a copy of the sanitized kernel UAPI header,
i.e. what you get as include/linux/ethtool.h by "make headers_install"
in kernel tree. (The copy in ethtool git is currently a bit behind but
the missing recent changes only modify comments so that it's not really
a problem.)

Modifying this file manually would mean that anyone who would update it
in the future (to sync with kernel changes) could not simply copy the
sanitized kernel header but would have to make sure to add your fragment
to it.

As you only need to define the __SANE_USERSPACE_TYPES__ macro (on
ppc64), it might be possible to achieve the same goal in Makefile.

Michal Kubecek

Re: ethtool - manual changes in ethtool-copy.h

[John W. Linville]

On Tue, Jan 29, 2019 at 09:28:42PM +0100, Michal Kubecek wrote:
> Hello,
> 
> I'm sorry I didn't notice earlier but ethtool commit 4df55c81996d
> ("ethtool: change to new sane powerpc64 kernel headers") adds changes to
> ethtool-copy.h which are not in sync with kernel file it is generated
> from.
> 
> This file is supposed to be a copy of the sanitized kernel UAPI header,
> i.e. what you get as include/linux/ethtool.h by "make headers_install"
> in kernel tree. (The copy in ethtool git is currently a bit behind but
> the missing recent changes only modify comments so that it's not really
> a problem.)
> 
> Modifying this file manually would mean that anyone who would update it
> in the future (to sync with kernel changes) could not simply copy the
> sanitized kernel header but would have to make sure to add your fragment
> to it.
> 
> As you only need to define the __SANE_USERSPACE_TYPES__ macro (on
> ppc64), it might be possible to achieve the same goal in Makefile.
> 
> Michal Kubecek

Ooops -- thanks for noticing that Michal!

Maciej, how soon might you be able to address this? What is the effect
of simply reverting it? Just warnings on ppc64 builds?

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

Re: ethtool - manual changes in ethtool-copy.h

[Maciej Żenczykowski]

On Tue, Jan 29, 2019 at 1:00 PM John W. Linville <linville@tuxdriver.com> wrote:
>
> On Tue, Jan 29, 2019 at 09:28:42PM +0100, Michal Kubecek wrote:
> > Hello,
> >
> > I'm sorry I didn't notice earlier but ethtool commit 4df55c81996d
> > ("ethtool: change to new sane powerpc64 kernel headers") adds changes to
> > ethtool-copy.h which are not in sync with kernel file it is generated
> > from.
> >
> > This file is supposed to be a copy of the sanitized kernel UAPI header,
> > i.e. what you get as include/linux/ethtool.h by "make headers_install"
> > in kernel tree. (The copy in ethtool git is currently a bit behind but
> > the missing recent changes only modify comments so that it's not really
> > a problem.)
> >
> > Modifying this file manually would mean that anyone who would update it
> > in the future (to sync with kernel changes) could not simply copy the
> > sanitized kernel header but would have to make sure to add your fragment
> > to it.
> >
> > As you only need to define the __SANE_USERSPACE_TYPES__ macro (on
> > ppc64), it might be possible to achieve the same goal in Makefile.
> >
> > Michal Kubecek
>
> Ooops -- thanks for noticing that Michal!
>
> Maciej, how soon might you be able to address this? What is the effect
> of simply reverting it? Just warnings on ppc64 builds?

Would this change be acceptable in the kernel header file this is
generated from?
Or would it mess something up?  I imagine it would be a problem... for
other users?
If so then could we just set this in ethtool Makefile... perhaps even
just blindly for all archs?

Re: ethtool - manual changes in ethtool-copy.h

[Maciej Żenczykowski]

Eh, I think this patch can simply be reverted.
This is a dupe.  ethtool-copy.h is only included from internal.h which
already includes:

...
#ifndef ETHTOOL_INTERNAL_H__
#define ETHTOOL_INTERNAL_H__

/* Some platforms (eg. ppc64) need __SANE_USERSPACE_TYPES__ before
 * <linux/types.h> to select 'int-ll64.h' and avoid compile warnings
 * when printing __u64 with %llu.
 */
#define __SANE_USERSPACE_TYPES__
...

which came in:

commit c0a2c04b3cbf6d399a2551654401957ddb529a50
Author: Maciej Żenczykowski <maze@google.com>
Date:   Fri Mar 11 09:58:14 2016 -0800

    internal.h: change to new sane kernel headers on 64-bit archs

    On ppc64, this fixes:
      ...
    Signed-off-by: Maciej Żenczykowski <maze@google.com>
    Signed-off-by: David Decotigny <decot@googlers.com>
    Signed-off-by: Ben Hutchings <ben@decadent.org.uk>

I think I missed this while trying to synchronize internal with upstream repos.
(the local copy of the patch was obviously in the wrong spot)

[PATCH] Revert "ethtool: change to new sane powerpc64 kernel headers"

[Maciej Żenczykowski]

From: Maciej Żenczykowski <maze@google.com>

This reverts commit 4df55c81996dfb1dbe98c93ee62d8067ed5073a9.

It turns out this is not needed due to:
    commit c0a2c04b3cbf6d399a2551654401957ddb529a50
    internal.h: change to new sane kernel headers on 64-bit archs
which I apparently entirely forgot about while trying
to synchronize internal and upstream git repositories.

Change-Id: I56d90a3c1e9b66c30526824fb7bc41aab01d85d1
Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 ethtool-copy.h | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/ethtool-copy.h b/ethtool-copy.h
index 7772a4970987..6bfbb85f9402 100644
--- a/ethtool-copy.h
+++ b/ethtool-copy.h
@@ -14,12 +14,6 @@
 #ifndef _LINUX_ETHTOOL_H
 #define _LINUX_ETHTOOL_H
 
-#ifdef __powerpc64__
-/* Powerpc needs __SANE_USERSPACE_TYPES__ before <linux/types.h> to select
- * 'int-ll64.h' and avoid compile warnings when printing __u64 with %llu.
- */
-#define __SANE_USERSPACE_TYPES__
-#endif
 #include <linux/kernel.h>
 #include <linux/types.h>
 #include <linux/if_ether.h>
-- 
2.20.1.495.gaa96b0ce6b-goog

Re: [PATCH] Revert "ethtool: change to new sane powerpc64 kernel headers"

[John W. Linville]

Cool, thanks -- queued for next release.

John

On Tue, Jan 29, 2019 at 06:48:49PM -0800, Maciej Żenczykowski wrote:
> From: Maciej Żenczykowski <maze@google.com>
> 
> This reverts commit 4df55c81996dfb1dbe98c93ee62d8067ed5073a9.
> 
> It turns out this is not needed due to:
>     commit c0a2c04b3cbf6d399a2551654401957ddb529a50
>     internal.h: change to new sane kernel headers on 64-bit archs
> which I apparently entirely forgot about while trying
> to synchronize internal and upstream git repositories.
> 
> Change-Id: I56d90a3c1e9b66c30526824fb7bc41aab01d85d1
> Signed-off-by: Maciej Żenczykowski <maze@google.com>
> ---
>  ethtool-copy.h | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/ethtool-copy.h b/ethtool-copy.h
> index 7772a4970987..6bfbb85f9402 100644
> --- a/ethtool-copy.h
> +++ b/ethtool-copy.h
> @@ -14,12 +14,6 @@
>  #ifndef _LINUX_ETHTOOL_H
>  #define _LINUX_ETHTOOL_H
>  
> -#ifdef __powerpc64__
> -/* Powerpc needs __SANE_USERSPACE_TYPES__ before <linux/types.h> to select
> - * 'int-ll64.h' and avoid compile warnings when printing __u64 with %llu.
> - */
> -#define __SANE_USERSPACE_TYPES__
> -#endif
>  #include <linux/kernel.h>
>  #include <linux/types.h>
>  #include <linux/if_ether.h>
> -- 
> 2.20.1.495.gaa96b0ce6b-goog
> 
> 

-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.