diff mbox series

[net] net: dsa: point out the tail taggers

Message ID 20201016162800.7696-1-ceggers@arri.de (mailing list archive)
State Not Applicable
Delegated to: Netdev Maintainers
Headers show
Series [net] net: dsa: point out the tail taggers | expand

Commit Message

Christian Eggers Oct. 16, 2020, 4:28 p.m. UTC 
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 <ceggers@arri.de>
---
 net/dsa/tag_ksz.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Florian Fainelli Oct. 16, 2020, 4:46 p.m. UTC | #1 
On 10/16/20 9:28 AM, 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 <ceggers@arri.de>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Vladimir Oltean Oct. 16, 2020, 4:56 p.m. UTC | #2 
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 <ceggers@arri.de>
> ---

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
kernel test robot Oct. 16, 2020, 5:25 p.m. UTC | #3 
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 <lkp@intel.com>

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/kbuild-all@lists.01.org
Vladimir Oltean Oct. 16, 2020, 5:33 p.m. UTC | #4 
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 <lkp@intel.com>
>
> 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/kbuild-all@lists.01.org

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
Andrew Lunn Oct. 16, 2020, 8:14 p.m. UTC | #5 
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
Vladimir Oltean Oct. 16, 2020, 8:19 p.m. UTC | #6 
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?
Andrew Lunn Oct. 16, 2020, 9:03 p.m. UTC | #7 
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
Vladimir Oltean Oct. 16, 2020, 9:16 p.m. UTC | #8 
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.
Vladimir Oltean Oct. 16, 2020, 9:33 p.m. UTC | #9 
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.
Jakub Kicinski Oct. 16, 2020, 10:56 p.m. UTC | #10 
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
diff mbox series

Patch

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);