On Thu, Dec 10, 2020 at 09:55:16AM -0800, Florian Fainelli wrote:
>
>
> On 12/10/2020 2:07 AM, Libing Zhou wrote:
> > During kernel startup phase, current netconsole doesn’t support VLAN
> > since there is no VLAN interface setup already.
> >
> > This patch provides VLAN ID and PCP as optional boot/module parameters
> > to support VLAN environment, thus kernel startup log can be retrieved
> > via VLAN.
> >
> > Signed-off-by: Libing Zhou <libing.zhou@xxxxxxxxxxxxxxx>
> > ---
> > Documentation/networking/netconsole.rst | 10 ++++-
> > drivers/net/netconsole.c | 3 +-
> > include/linux/netpoll.h | 3 ++
> > net/core/netpoll.c | 58 ++++++++++++++++++++++++-
> > 4 files changed, 70 insertions(+), 4 deletions(-)
> >
> > diff --git a/Documentation/networking/netconsole.rst b/Documentation/networking/netconsole.rst
> > index 1f5c4a04027c..a08387fcc3f0 100644
> > --- a/Documentation/networking/netconsole.rst
> > +++ b/Documentation/networking/netconsole.rst
> > @@ -13,6 +13,8 @@ IPv6 support by Cong Wang <xiyou.wangcong@xxxxxxxxx>, Jan 1 2013
> >
> > Extended console support by Tejun Heo <tj@xxxxxxxxxx>, May 1 2015
> >
> > +VLAN support by Libing Zhou <libing.zhou@xxxxxxxxxxxxxxx>, Dec 8 2020
> > +
> > Please send bug reports to Matt Mackall <mpm@xxxxxxxxxxx>
> > Satyam Sharma <satyam.sharma@xxxxxxxxx>, and Cong Wang <xiyou.wangcong@xxxxxxxxx>
> >
> > @@ -34,7 +36,7 @@ Sender and receiver configuration:
> > It takes a string configuration parameter "netconsole" in the
> > following format::
> >
> > - netconsole=[+][src-port]@[src-ip]/[<dev>],[tgt-port]@<tgt-ip>/[tgt-macaddr]
> > + netconsole=[+][src-port]@[src-ip]/[<dev>],[tgt-port]@<tgt-ip>/[tgt-macaddr][-V<vid:pcp>]
> >
> > where
> > + if present, enable extended console support
> > @@ -44,11 +46,17 @@ following format::
> > tgt-port port for logging agent (6666)
> > tgt-ip IP address for logging agent
> > tgt-macaddr ethernet MAC address for logging agent (broadcast)
> > + -V if present, enable VLAN support
> > + vid:pcp VLAN identifier and priority code point
> >
> > Examples::
> >
> > linux netconsole=4444@10.0.0.1/eth1,9353@10.0.0.2/12:34:56:78:9a:bc
> >
> > +or using VLAN::
> > +
> > + linux netconsole=4444@10.0.0.1/eth1,9353@10.0.0.2/12:34:56:78:9a:bc-V100:1
> > +
> > or::
> >
> > insmod netconsole netconsole=@/,@10.0.0.2/
> > diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
> > index 92001f7af380..f0690cd6a744 100644
> > --- a/drivers/net/netconsole.c
> > +++ b/drivers/net/netconsole.c
> > @@ -36,7 +36,6 @@
> > #include <linux/inet.h>
> > #include <linux/configfs.h>
> > #include <linux/etherdevice.h>
> > -
> > MODULE_AUTHOR("Maintainer: Matt Mackall <mpm@xxxxxxxxxxx>");
> > MODULE_DESCRIPTION("Console driver for network interfaces");
> > MODULE_LICENSE("GPL");
> > @@ -46,7 +45,7 @@ MODULE_LICENSE("GPL");
> >
> > static char config[MAX_PARAM_LENGTH];
> > module_param_string(netconsole, config, MAX_PARAM_LENGTH, 0);
> > -MODULE_PARM_DESC(netconsole, " netconsole=[src-port]@[src-ip]/[dev],[tgt-port]@<tgt-ip>/[tgt-macaddr]");
> > +MODULE_PARM_DESC(netconsole, " netconsole=[src-port]@[src-ip]/[dev],[tgt-port]@<tgt-ip>/[tgt-macaddr][-V<vid:pcp>]");
> >
> > static bool oops_only = false;
> > module_param(oops_only, bool, 0600);
> > diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h
> > index e6a2d72e0dc7..8ab3f25cadae 100644
> > --- a/include/linux/netpoll.h
> > +++ b/include/linux/netpoll.h
> > @@ -31,6 +31,9 @@ struct netpoll {
> > bool ipv6;
> > u16 local_port, remote_port;
> > u8 remote_mac[ETH_ALEN];
> > + bool vlan_present;
> > + u16 vlan_id;
> > + u8 pcp;
> > };
> >
> > struct netpoll_info {
> > diff --git a/net/core/netpoll.c b/net/core/netpoll.c
> > index 2338753e936b..077a7aec51ae 100644
> > --- a/net/core/netpoll.c
> > +++ b/net/core/netpoll.c
> > @@ -478,6 +478,14 @@ void netpoll_send_udp(struct netpoll *np, const char *msg, int len)
> >
> > skb->dev = np->dev;
> >
> > + if (np->vlan_present) {
> > + skb->vlan_proto = htons(ETH_P_8021Q);
> > +
> > + /* htons for tci is done in __vlan_insert_inner_tag, not here */
> > + skb->vlan_tci = (np->pcp << VLAN_PRIO_SHIFT) + (np->vlan_id & VLAN_VID_MASK);
> > + skb->vlan_present = 1;
> > + }
>
> This does not seem to be the way to go around this, I would rather
> specifying eth0.<VID> on the netconsole parameters and automatically
> create a VLAN interface from that which would ensure that everything
> works properly and that the VLAN interface is linked to its lower device
> properly.
>
> If you prefer your current syntax, that is probably fine, too but you
> should consider registering a VLAN device when you parse appropriate
> options.
> --
> Florian
I don't have a strong opinion, considering that I have not actually
tried this. I do tend to agree with Florian about registering an 8021q
interface, as long as there aren't any other complications associated
with it. As for the syntax, I am not sure if "eth0.<VID>" is universal
enough to make an ABI out of it.
