2020-10-16 18:55:53

by Christian Eggers

[permalink] [raw]
Subject: [PATCH net] net: dsa: point out the tail taggers

From a recent commit with the same summary:

"The Marvell 88E6060 uses tag_trailer.c and the KSZ8795, KSZ9477 and
KSZ9893 switches also use tail tags."

Set "tail_tag" to true for KSZ8795 and KSZ9477 which were missing in the
original commit.

Fixes: 7a6ffe764be3 [net] ("net: dsa: point out the tail taggers")
Signed-off-by: Christian Eggers <[email protected]>
---
net/dsa/tag_ksz.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/net/dsa/tag_ksz.c b/net/dsa/tag_ksz.c
index 945a9bd5ba35..0a5aa982c60d 100644
--- a/net/dsa/tag_ksz.c
+++ b/net/dsa/tag_ksz.c
@@ -123,6 +123,7 @@ static const struct dsa_device_ops ksz8795_netdev_ops = {
.xmit = ksz8795_xmit,
.rcv = ksz8795_rcv,
.overhead = KSZ_INGRESS_TAG_LEN,
+ .tail_tag = true,
};

DSA_TAG_DRIVER(ksz8795_netdev_ops);
@@ -199,6 +200,7 @@ static const struct dsa_device_ops ksz9477_netdev_ops = {
.xmit = ksz9477_xmit,
.rcv = ksz9477_rcv,
.overhead = KSZ9477_INGRESS_TAG_LEN,
+ .tail_tag = true,
};

DSA_TAG_DRIVER(ksz9477_netdev_ops);
--
Christian Eggers
Embedded software developer

Arnold & Richter Cine Technik GmbH & Co. Betriebs KG
Sitz: Muenchen - Registergericht: Amtsgericht Muenchen - Handelsregisternummer: HRA 57918
Persoenlich haftender Gesellschafter: Arnold & Richter Cine Technik GmbH
Sitz: Muenchen - Registergericht: Amtsgericht Muenchen - Handelsregisternummer: HRB 54477
Geschaeftsfuehrer: Dr. Michael Neuhaeuser; Stephan Schenk; Walter Trauninger; Markus Zeiler


2020-10-16 19:05:37

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH net] net: dsa: point out the tail taggers

Hi Christian,

On Fri, Oct 16, 2020 at 06:28:00PM +0200, Christian Eggers wrote:
> From a recent commit with the same summary:
>
> "The Marvell 88E6060 uses tag_trailer.c and the KSZ8795, KSZ9477 and
> KSZ9893 switches also use tail tags."
>
> Set "tail_tag" to true for KSZ8795 and KSZ9477 which were missing in the
> original commit.
>
> Fixes: 7a6ffe764be3 [net] ("net: dsa: point out the tail taggers")
> Signed-off-by: Christian Eggers <[email protected]>
> ---

The idea is perfect but the commit isn't.

First of all, put this in your .gitconfig.

[core]
abbrev = 12
[pretty]
fixes = Fixes: %h (\"%s\")

Now if you run
"git show 7a6ffe764be35af0527d8cfd047945e8f8797ddf --pretty=fixes",
you'll see:

Fixes: 7a6ffe764be3 ("net: dsa: point out the tail taggers")

Notice how there's no [net] tag?
People complain when the format of the Fixes: tag is not standardized.

Secondly, can you please come up with a commit description that is
_different_ from the commit you're fixing? As a backporter I would hate
to have 2 commits with the same title, I would surely mess them up.

How about:
net: dsa: tag_ksz: KSZ8795 and KSZ9477 also use tail tags

2020-10-16 19:58:23

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH net] net: dsa: point out the tail taggers

On Sat, Oct 17, 2020 at 01:25:08AM +0800, kernel test robot wrote:
> Hi Christian,
>
> Thank you for the patch! Yet something to improve:
>
> [auto build test ERROR on net/master]
>
> url: https://github.com/0day-ci/linux/commits/Christian-Eggers/net-dsa-point-out-the-tail-taggers/20201017-003007
> base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git 2ecbc1f684482b4ed52447a39903bd9b0f222898
> config: xtensa-allyesconfig (attached as .config)
> compiler: xtensa-linux-gcc (GCC) 9.3.0
> reproduce (this is a W=1 build):
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # https://github.com/0day-ci/linux/commit/aaa07cad29bf365264beb2c2e2668db83ca31923
> git remote add linux-review https://github.com/0day-ci/linux
> git fetch --no-tags linux-review Christian-Eggers/net-dsa-point-out-the-tail-taggers/20201017-003007
> git checkout aaa07cad29bf365264beb2c2e2668db83ca31923
> # save the attached .config to linux build tree
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=xtensa
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <[email protected]>
>
> All error/warnings (new ones prefixed by >>):
>
> >> net/dsa/tag_ksz.c:126:3: error: 'const struct dsa_device_ops' has no member named 'tail_tag'
> 126 | .tail_tag = true,
> | ^~~~~~~~
> >> net/dsa/tag_ksz.c:126:14: warning: initialization of 'const char *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
> 126 | .tail_tag = true,
> | ^~~~
> net/dsa/tag_ksz.c:126:14: note: (near initialization for 'ksz8795_netdev_ops.name')
> >> net/dsa/tag_ksz.c:126:14: warning: initialized field overwritten [-Woverride-init]
> net/dsa/tag_ksz.c:126:14: note: (near initialization for 'ksz8795_netdev_ops.name')
> net/dsa/tag_ksz.c:203:3: error: 'const struct dsa_device_ops' has no member named 'tail_tag'
> 203 | .tail_tag = true,
> | ^~~~~~~~
> net/dsa/tag_ksz.c:203:14: warning: initialization of 'const char *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
> 203 | .tail_tag = true,
> | ^~~~
> net/dsa/tag_ksz.c:203:14: note: (near initialization for 'ksz9477_netdev_ops.name')
> net/dsa/tag_ksz.c:203:14: warning: initialized field overwritten [-Woverride-init]
> net/dsa/tag_ksz.c:203:14: note: (near initialization for 'ksz9477_netdev_ops.name')
>
> vim +126 net/dsa/tag_ksz.c
>
> 119
> 120 static const struct dsa_device_ops ksz8795_netdev_ops = {
> 121 .name = "ksz8795",
> 122 .proto = DSA_TAG_PROTO_KSZ8795,
> 123 .xmit = ksz8795_xmit,
> 124 .rcv = ksz8795_rcv,
> 125 .overhead = KSZ_INGRESS_TAG_LEN,
> > 126 .tail_tag = true,
> 127 };
> 128
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/[email protected]

Is the test bot being a bit "slow" today?
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/tree/include/net/dsa.h#n93?id=2ecbc1f684482b4ed52447a39903bd9b0f222898

2020-10-16 20:19:46

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH net] net: dsa: point out the tail taggers

On Fri, Oct 16, 2020 at 08:33:17PM +0300, Vladimir Oltean wrote:
> On Sat, Oct 17, 2020 at 01:25:08AM +0800, kernel test robot wrote:
> > Hi Christian,
> >
> > Thank you for the patch! Yet something to improve:
> >
> > [auto build test ERROR on net/master]
> >
> > url: https://github.com/0day-ci/linux/commits/Christian-Eggers/net-dsa-point-out-the-tail-taggers/20201017-003007
> > base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git 2ecbc1f684482b4ed52447a39903bd9b0f222898

> Is the test bot being a bit "slow" today?

It is using the net.git commit from yesterday afternoon. net-next got
merged into net yesterday evening, so it is a bit behind, but not too
far behind.

Andrew

2020-10-16 20:23:42

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH net] net: dsa: point out the tail taggers

On Fri, Oct 16, 2020 at 10:14:28PM +0200, Andrew Lunn wrote:
> On Fri, Oct 16, 2020 at 08:33:17PM +0300, Vladimir Oltean wrote:
> > On Sat, Oct 17, 2020 at 01:25:08AM +0800, kernel test robot wrote:
> > > Hi Christian,
> > >
> > > Thank you for the patch! Yet something to improve:
> > >
> > > [auto build test ERROR on net/master]
> > >
> > > url: https://github.com/0day-ci/linux/commits/Christian-Eggers/net-dsa-point-out-the-tail-taggers/20201017-003007
> > > base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git 2ecbc1f684482b4ed52447a39903bd9b0f222898
>
> > Is the test bot being a bit "slow" today?
>
> It is using the net.git commit from yesterday afternoon. net-next got
> merged into net yesterday evening, so it is a bit behind, but not too
> far behind.

Exactly. The sha1sum it uses _does_ contain the tail_tag member inside
struct dsa_device_ops. What's it complaining about?

2020-10-16 20:55:53

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH net] net: dsa: point out the tail taggers

Hi Christian,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net/master]

url: https://github.com/0day-ci/linux/commits/Christian-Eggers/net-dsa-point-out-the-tail-taggers/20201017-003007
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git 2ecbc1f684482b4ed52447a39903bd9b0f222898
config: xtensa-allyesconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/aaa07cad29bf365264beb2c2e2668db83ca31923
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Christian-Eggers/net-dsa-point-out-the-tail-taggers/20201017-003007
git checkout aaa07cad29bf365264beb2c2e2668db83ca31923
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=xtensa

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All error/warnings (new ones prefixed by >>):

>> net/dsa/tag_ksz.c:126:3: error: 'const struct dsa_device_ops' has no member named 'tail_tag'
126 | .tail_tag = true,
| ^~~~~~~~
>> net/dsa/tag_ksz.c:126:14: warning: initialization of 'const char *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
126 | .tail_tag = true,
| ^~~~
net/dsa/tag_ksz.c:126:14: note: (near initialization for 'ksz8795_netdev_ops.name')
>> net/dsa/tag_ksz.c:126:14: warning: initialized field overwritten [-Woverride-init]
net/dsa/tag_ksz.c:126:14: note: (near initialization for 'ksz8795_netdev_ops.name')
net/dsa/tag_ksz.c:203:3: error: 'const struct dsa_device_ops' has no member named 'tail_tag'
203 | .tail_tag = true,
| ^~~~~~~~
net/dsa/tag_ksz.c:203:14: warning: initialization of 'const char *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
203 | .tail_tag = true,
| ^~~~
net/dsa/tag_ksz.c:203:14: note: (near initialization for 'ksz9477_netdev_ops.name')
net/dsa/tag_ksz.c:203:14: warning: initialized field overwritten [-Woverride-init]
net/dsa/tag_ksz.c:203:14: note: (near initialization for 'ksz9477_netdev_ops.name')

vim +126 net/dsa/tag_ksz.c

119
120 static const struct dsa_device_ops ksz8795_netdev_ops = {
121 .name = "ksz8795",
122 .proto = DSA_TAG_PROTO_KSZ8795,
123 .xmit = ksz8795_xmit,
124 .rcv = ksz8795_rcv,
125 .overhead = KSZ_INGRESS_TAG_LEN,
> 126 .tail_tag = true,
127 };
128

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]


Attachments:
(No filename) (2.86 kB)
.config.gz (63.52 kB)
Download all attachments

2020-10-16 22:33:15

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH net] net: dsa: point out the tail taggers

On Fri, Oct 16, 2020 at 11:03:18PM +0200, Andrew Lunn wrote:
> 2ecbc1f684482b4ed52447a39903bd9b0f222898 does not have net-next, as
> far as i see,

Not sure what you mean by that.

> and tail_tag only hit net when net-next was merged into
> net.

net-next is only merged into net via Linus Torvalds, as far as I
understand.

> Or i'm reading the git history wrong.

So the only plausible scenario is that yesterday's 'net/master' did not
contain 2ecbc1f684482b4ed52447a39903bd9b0f222898, but today it does, due
to Linus Torvalds merging net-next and Jakub merging that merge into net.

2020-10-16 22:34:28

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH net] net: dsa: point out the tail taggers

On Sat, Oct 17, 2020 at 12:16:28AM +0300, Vladimir Oltean wrote:
> On Fri, Oct 16, 2020 at 11:03:18PM +0200, Andrew Lunn wrote:
> > 2ecbc1f684482b4ed52447a39903bd9b0f222898 does not have net-next, as
> > far as i see,
>
> Not sure what you mean by that.

Ah, I do understand what you mean now. In git, that is what I see as
well. But in my cgit link, why would tail_tag be there?
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/tree/include/net/dsa.h#n93?id=2ecbc1f684482b4ed52447a39903bd9b0f222898
I think either cgit is plainly dumb at showing the kernel tree at a
particular commit, or I'm plainly incapable of using it.

2020-10-16 22:34:37

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH net] net: dsa: point out the tail taggers

On Fri, Oct 16, 2020 at 11:19:30PM +0300, Vladimir Oltean wrote:
> On Fri, Oct 16, 2020 at 10:14:28PM +0200, Andrew Lunn wrote:
> > On Fri, Oct 16, 2020 at 08:33:17PM +0300, Vladimir Oltean wrote:
> > > On Sat, Oct 17, 2020 at 01:25:08AM +0800, kernel test robot wrote:
> > > > Hi Christian,
> > > >
> > > > Thank you for the patch! Yet something to improve:
> > > >
> > > > [auto build test ERROR on net/master]
> > > >
> > > > url: https://github.com/0day-ci/linux/commits/Christian-Eggers/net-dsa-point-out-the-tail-taggers/20201017-003007
> > > > base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git 2ecbc1f684482b4ed52447a39903bd9b0f222898
> >
> > > Is the test bot being a bit "slow" today?
> >
> > It is using the net.git commit from yesterday afternoon. net-next got
> > merged into net yesterday evening, so it is a bit behind, but not too
> > far behind.
>
> Exactly. The sha1sum it uses _does_ contain the tail_tag member inside
> struct dsa_device_ops. What's it complaining about?

2ecbc1f684482b4ed52447a39903bd9b0f222898 does not have net-next, as
far as i see, and tail_tag only hit net when net-next was merged into
net.

Or i'm reading the git history wrong.

Andrew

2020-10-17 07:37:05

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH net] net: dsa: point out the tail taggers

On Sat, 17 Oct 2020 00:33:02 +0300 Vladimir Oltean wrote:
> On Sat, Oct 17, 2020 at 12:16:28AM +0300, Vladimir Oltean wrote:
> > On Fri, Oct 16, 2020 at 11:03:18PM +0200, Andrew Lunn wrote:
> > > 2ecbc1f684482b4ed52447a39903bd9b0f222898 does not have net-next, as
> > > far as i see,
> >
> > Not sure what you mean by that.
>
> Ah, I do understand what you mean now. In git, that is what I see as
> well. But in my cgit link, why would tail_tag be there?
> https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/tree/include/net/dsa.h#n93?id=2ecbc1f684482b4ed52447a39903bd9b0f222898
> I think either cgit is plainly dumb at showing the kernel tree at a
> particular commit, or I'm plainly incapable of using it.

The link is bamboozled.

The #n93 needs to be after the ? parameters.

Like this:

https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/tree/include/net/dsa.h?id=2ecbc1f684482b4ed52447a39903bd9b0f222898#n86