netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Florian Fainelli <f.fainelli@gmail.com>
To: Vladimir Oltean <olteanv@gmail.com>
Cc: netdev@vger.kernel.org, Andrew Lunn <andrew@lunn.ch>,
	Vivien Didelot <vivien.didelot@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>, Jiri Pirko <jiri@mellanox.com>,
	Eric Dumazet <edumazet@google.com>,
	Taehee Yoo <ap420073@gmail.com>,
	Cong Wang <xiyou.wangcong@gmail.com>,
	Maxim Mikityanskiy <maximmi@mellanox.com>,
	Richard Cochran <richardcochran@gmail.com>,
	Michal Kubecek <mkubecek@suse.cz>,
	open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH net-next 3/4] net: Call into DSA netdevice_ops wrappers
Date: Sat, 18 Jul 2020 14:53:27 -0700	[thread overview]
Message-ID: <d3a11ef5-3e4e-0c4a-790b-63da94a1545c@gmail.com> (raw)
In-Reply-To: <20200718211805.3yyckq23udacz4sa@skbuf>



On 7/18/2020 2:18 PM, Vladimir Oltean wrote:
> On Fri, Jul 17, 2020 at 08:05:32PM -0700, Florian Fainelli wrote:
>> Make the core net_device code call into our ndo_do_ioctl() and
>> ndo_get_phys_port_name() functions via the wrappers defined previously
>>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>>  net/core/dev.c       | 5 +++++
>>  net/core/dev_ioctl.c | 5 +++++
>>  2 files changed, 10 insertions(+)
>>
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index 062a00fdca9b..19f1abc26fcd 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -98,6 +98,7 @@
>>  #include <net/busy_poll.h>
>>  #include <linux/rtnetlink.h>
>>  #include <linux/stat.h>
>> +#include <net/dsa.h>
>>  #include <net/dst.h>
>>  #include <net/dst_metadata.h>
>>  #include <net/pkt_sched.h>
>> @@ -8602,6 +8603,10 @@ int dev_get_phys_port_name(struct net_device *dev,
>>  	const struct net_device_ops *ops = dev->netdev_ops;
>>  	int err;
>>  
>> +	err  = dsa_ndo_get_phys_port_name(dev, name, len);
> 
> Stupid question, but why must these be calls to an inline function whose
> name is derived through macro concatenation and hardcoded for 2
> arguments, then pass through an additional function pointer found in a
> DSA-specific lookup table, and why cannot DSA instead simply export
> these 2 symbols (with a static inline EOPNOTSUPP fallback), and simply
> provide the implementation inside those?

The macros could easily be changed to take a single argument list and
play tricks with arguments ordering etc. so I would decouple them from
the choice of using them.

If we have the core network stack reference DSA as a module then we
force DSA to be either built-in or not, which is not very practical,
people would still want a modular choice to be possible. The static
inline only wraps indirect function pointer calls using definitions
available at build time and actual function pointer substitution at run
time, so we avoid that problem entirely that way.
-- 
Florian

  reply	other threads:[~2020-07-18 21:53 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-18  3:05 [PATCH net-next 0/4] net: dsa: Setup dsa_netdev_ops Florian Fainelli
2020-07-18  3:05 ` [PATCH net-next 1/4] net: Wrap ndo_do_ioctl() to prepare for DSA stacked ops Florian Fainelli
2020-07-18 20:30   ` Vladimir Oltean
2020-07-18 20:36     ` Florian Fainelli
2020-07-18 20:44       ` Vladimir Oltean
2020-07-19 15:31   ` Andrew Lunn
2020-07-18  3:05 ` [PATCH net-next 2/4] net: dsa: Add wrappers for overloaded ndo_ops Florian Fainelli
2020-07-18  4:53   ` kernel test robot
2020-07-18 18:53     ` Florian Fainelli
2020-07-18 20:18       ` Vladimir Oltean
2020-07-18 20:20         ` Florian Fainelli
2020-07-19 15:40   ` Andrew Lunn
2020-07-19 16:10     ` Florian Fainelli
2020-07-18  3:05 ` [PATCH net-next 3/4] net: Call into DSA netdevice_ops wrappers Florian Fainelli
2020-07-18 21:18   ` Vladimir Oltean
2020-07-18 21:53     ` Florian Fainelli [this message]
2020-07-19 16:04       ` Andrew Lunn
2020-07-19 16:08         ` Florian Fainelli
2020-07-18  3:05 ` [PATCH net-next 4/4] net: dsa: Setup dsa_netdev_ops Florian Fainelli

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=d3a11ef5-3e4e-0c4a-790b-63da94a1545c@gmail.com \
    --to=f.fainelli@gmail.com \
    --cc=andrew@lunn.ch \
    --cc=ap420073@gmail.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=jiri@mellanox.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maximmi@mellanox.com \
    --cc=mkubecek@suse.cz \
    --cc=netdev@vger.kernel.org \
    --cc=olteanv@gmail.com \
    --cc=richardcochran@gmail.com \
    --cc=vivien.didelot@gmail.com \
    --cc=xiyou.wangcong@gmail.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).