From: Jakub Kicinski <kuba@kernel.org>
To: Fernando Fernandez Mancera <fmancera@suse.de>
Cc: "Ján Václav" <jvaclav@redhat.com>,
"David S. Miller" <davem@davemloft.net>,
"Eric Dumazet" <edumazet@google.com>,
"Paolo Abeni" <pabeni@redhat.com>,
"Simon Horman" <horms@kernel.org>,
netdev@vger.kernel.org
Subject: Re: [PATCH v2 net-next] net/hsr: add protocol version to fill_info output
Date: Thu, 25 Sep 2025 19:00:12 -0700 [thread overview]
Message-ID: <20250925190012.58e1b3b1@kernel.org> (raw)
In-Reply-To: <c39e6626-02db-4a83-9f77-3d661f63ac0e@suse.de>
On Thu, 25 Sep 2025 10:37:38 +0200 Fernando Fernandez Mancera wrote:
> > I'm not very familiar with HSR or PRP. But The PRP_V1 which has value
> > of 3 looks like a kernel-internal hack. Or does the protocol actually
> > specify value 3 to mean PRP?
> >
> > I don't think there's anything particularly wrong with the code.
> > The version is for HSR because PRP only has one version, there's no
> > ambiguity.
> >
> > But again, I'm just glancing at the code I could be wrong..
> >
>
> No you are right, this is a hack made to integrate PRP with HSR driver.
> PRP does not have a version other than PRP_V1 therefore it does not make
> much sense to configure it. Having said that, I think it's weird to
> report HSR_VERSION 3 but fail when configuring it.
>
> IMHO HSR_VERSION should be hidden for PRP or it should be possible to
> configure it to "3" (which now that you say it, it looks weird).
I think we're in agreement then? i was suggesting:
--- a/net/hsr/hsr_netlink.c
+++ b/net/hsr/hsr_netlink.c
@@ -166,6 +166,8 @@ static int hsr_fill_info(struct sk_buff *skb, const struct net_device *dev)
goto nla_put_failure;
if (hsr->prot_version == PRP_V1)
proto = HSR_PROTOCOL_PRP;
+ else if (nla_put_u8(skb, IFLA_HSR_VERSION, hsr->prot_version))
+ goto nla_put_failure;
if (nla_put_u8(skb, IFLA_HSR_PROTOCOL, proto))
goto nla_put_failure;
This will not report the HSR version if prot is PRP
next prev parent reply other threads:[~2025-09-26 2:00 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-09-22 9:37 [PATCH v2 net-next] net/hsr: add protocol version to fill_info output Jan Vaclav
2025-09-23 7:22 ` Simon Horman
2025-09-24 0:06 ` Jakub Kicinski
2025-09-24 11:21 ` Ján Václav
2025-09-24 12:01 ` Fernando Fernandez Mancera
2025-09-24 23:40 ` Jakub Kicinski
2025-09-25 8:37 ` Fernando Fernandez Mancera
2025-09-26 2:00 ` Jakub Kicinski [this message]
2025-09-26 9:33 ` Fernando Fernandez Mancera
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250925190012.58e1b3b1@kernel.org \
--to=kuba@kernel.org \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=fmancera@suse.de \
--cc=horms@kernel.org \
--cc=jvaclav@redhat.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.