netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Robert Shearman <rshearma@brocade.com>
To: Jiri Benc <jbenc@redhat.com>
Cc: <davem@davemloft.net>, <netdev@vger.kernel.org>,
	Roopa Prabhu <roopa@cumulusnetworks.com>,
	Tom Herbert <tom@herbertland.com>, Thomas Graf <tgraf@suug.ch>,
	"Eric W. Biederman" <ebiederm@xmission.com>
Subject: Re: [PATCH net-next 1/3] lwtunnel: autoload of lwt modules
Date: Mon, 15 Feb 2016 18:08:13 +0000	[thread overview]
Message-ID: <56C2140D.9090708@brocade.com> (raw)
In-Reply-To: <20160215173209.055b7b6b@griffin>

On 15/02/16 16:32, Jiri Benc wrote:
> On Mon, 15 Feb 2016 16:22:08 +0000, Robert Shearman wrote:
>> Yeah, it's the C preprocessor. MODULE_ALIAS_RTNL_LWT includes the string
>> for the encap type in the module alias, and since the LWT encap types
>> are defined as enums this is symbolic name. I can't see any way of
>> getting the preprocessor to convert
>> MODULE_ALIAS_RTNL_LWT(LWTUNNEL_ENCAP_MPLS) into "rtnl-lwt-MPLS", but I'm
>> open to suggestions.
>
> MODULE_ALIAS_RTNL_LWT(MPLS)?
>
> But whatever, as I said, no strong preference.

I was so hung up on the making the string match the name of the enum 
that I'd discounted that, but you're right that doing that would reduce 
duplication in the alias string.

>> True, but I figured that it was cleaner for the lwtunnel infra to not
>> assume whether how those modules are implemented. If you disagree, then
>> I can change to doing as you suggest.
>
> It's not completely transparent to the infrastructure anyway, the
> tunnel type needs to be added to lwtunnel_encap_str for new tunnels.
> The way I suggested, it's only added for those tunnels having the
> module alias set.
>
> Just trying to get rid of the unnecessary strings in
> lwtunnel_encap_str. There's no need to bloat kernel with them if
> they're never used.

Ok, will resubmit without the unnecessary strings in that function as 
well as with your suggestion above.

Thanks for the review,
Rob

  reply	other threads:[~2016-02-15 18:08 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-15 15:42 [PATCH net-next 0/3] lwtunnel: autoload of lwt modules Robert Shearman
2016-02-15 15:42 ` [PATCH net-next 1/3] " Robert Shearman
2016-02-15 16:02   ` Jiri Benc
2016-02-15 16:22     ` Robert Shearman
2016-02-15 16:32       ` Jiri Benc
2016-02-15 18:08         ` Robert Shearman [this message]
2016-02-15 21:33   ` Eric W. Biederman
2016-02-16 14:14     ` Robert Shearman
2016-02-15 15:42 ` [PATCH net-next 2/3] mpls: autoload lwt module Robert Shearman
2016-02-15 15:42 ` [PATCH net-next 3/3] ila: autoload module Robert Shearman
2016-02-19  9:43 ` [PATCH net-next v2 0/3] lwtunnel: autoload of lwt modules Robert Shearman
2016-02-19  9:43   ` [PATCH net-next v2 1/3] " Robert Shearman
2016-02-19  9:43   ` [PATCH net-next v2 2/3] mpls: autoload lwt module Robert Shearman
2016-02-19  9:43   ` [PATCH net-next v2 3/3] ila: autoload module Robert Shearman
2016-02-22  3:00   ` [PATCH net-next v2 0/3] lwtunnel: autoload of lwt modules David Miller

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=56C2140D.9090708@brocade.com \
    --to=rshearma@brocade.com \
    --cc=davem@davemloft.net \
    --cc=ebiederm@xmission.com \
    --cc=jbenc@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=roopa@cumulusnetworks.com \
    --cc=tgraf@suug.ch \
    --cc=tom@herbertland.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).