* Re: [PATCH] xfrm: don't segment UFO packets
From: Steffen Klassert @ 2016-03-18 8:05 UTC (permalink / raw)
To: Herbert Xu; +Cc: Jiri Bohac, David S. Miller, netdev
In-Reply-To: <20160318023653.GA17000@gondor.apana.org.au>
On Fri, Mar 18, 2016 at 10:36:53AM +0800, Herbert Xu wrote:
> On Thu, Mar 17, 2016 at 06:08:55PM +0100, Jiri Bohac wrote:
> > On Thu, Mar 17, 2016 at 11:24:59AM +0100, Steffen Klassert wrote:
> > > In IPv6 this check is missing, so this could be the
> > > problem if this is IPv6.
> >
> > indeed, this patch also fixes my problem:
>
> Hmm, is this what you really want? If I understood you correctly,
> you want the fragmentation to occur after IPsec.
The main problem was probably that UFO handling does not work at
all on IPv6 IPsec.
> So while this
> might generate the same output, it is still going to prefragment
> the data, only to merge them back for IPsec and then refragment
> again.
This is far away from being optimal, but this is what usually
happens if a local application sends data that we need to
fragment.
We currently work on avoiding the linearization on IPsec,
but having a skb with a fraglist is really the worst case.
^ permalink raw reply
* Re: [PATCH net-next] virtio_net: replace netdev_alloc_skb_ip_align() with napi_alloc_skb()
From: Paolo Abeni @ 2016-03-18 8:21 UTC (permalink / raw)
To: Venkatesh Srinivas
Cc: netdev, Michael S. Tsirkin, Hannes Frederic Sowa, virtualization
In-Reply-To: <CAHdzE-_u35Kg1zck1KKCYhu5epx4vNtzE7UX41Bi85vsJ55ROw@mail.gmail.com>
On Thu, 2016-03-17 at 18:01 -0700, Venkatesh Srinivas wrote:
> On Thu, Mar 17, 2016 at 7:44 AM, Paolo Abeni <pabeni@redhat.com> wrote:
> >
> > This gives small but noticeable rx performance improvement (2-3%)
> > and will allow exploiting future napi improvement.
> >
> > Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> > ---
>
> Worked in my tests, though the performance win was in the noise (+0.6%
> - +1% bandwidth).
> What tests did you see a 2-3% win on?
I'm doing net2VM tests (i.e. the traffic generator is on a different
host) with guest tuned a bit for performance. i.e. cpu pinning, no
iptables, no dhclient (yes, that makes a big difference:
http://lists.openwall.net/netdev/2013/10/29/5)
> Do you think its worth modifying add_recvbuf_small() to use
> napi_alloc_skb() when called from
> Rx NAPI (virtnet_receive)?
Oops, I missed that invocation. Probably that path is not hit in my
test. I think it should be really worthy.
I'll send a v2.
The important thing, more than the current improvement, is allowing the
virtio_net driver to leverage future napi allocation improvement (i.e.
bulk alloc: http://www.spinics.net/lists/netdev/msg367568.html) which
should be a real win.
Paolo
^ permalink raw reply
* [PATCH] net: consolidate lock/unlock into unlock_wait
From: Nicholas Mc Guire @ 2016-03-18 9:32 UTC (permalink / raw)
To: Joe Perches; +Cc: David S. Miller, netdev, linux-kernel, Nicholas Mc Guire
The spin_lock()/spin_unlock() is synchronizing on the adapter->work_lock
as the comment also suggests, which is equivalent to spin_unlock_wait()
but the later should be more efficient.
Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
---
Problem located by coccinelle spatch
Patch was compile tested with: x86_64_defconfig + CONFIG_CHELSIO_T1=m
Patch is against linux-next (localversion-next is next-20160318)
drivers/net/ethernet/chelsio/cxgb/cxgb2.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb/cxgb2.c b/drivers/net/ethernet/chelsio/cxgb/cxgb2.c
index f5f1b0b..c5a38dc 100644
--- a/drivers/net/ethernet/chelsio/cxgb/cxgb2.c
+++ b/drivers/net/ethernet/chelsio/cxgb/cxgb2.c
@@ -282,8 +282,8 @@ static int cxgb_close(struct net_device *dev)
!(adapter->open_device_map & PORT_MASK)) {
/* Stop statistics accumulation. */
smp_mb__after_atomic();
- spin_lock(&adapter->work_lock); /* sync with update task */
- spin_unlock(&adapter->work_lock);
+ /* sync with update task */
+ spin_unlock_wait(&adapter->work_lock);
cancel_mac_stats_update(adapter);
}
--
2.1.4
^ permalink raw reply related
* Re: [PATCH] qeth: Default to allow promiscuous mode
From: Alexander Graf @ 2016-03-18 9:36 UTC (permalink / raw)
To: Evgeny Cherkashin
Cc: linux-s390, linux-kernel, heicars2, mschwid2, Ursula Braun, ihno,
mt, pwieczorkiewicz, netdev@vger.kernel.org
In-Reply-To: <201603171752.u2HHq7hM025196@d06av11.portsmouth.uk.ibm.com>
On 17.03.16 18:52, Evgeny Cherkashin wrote:
> Hello all,
>
> -----Alexander Graf <agraf@suse.de> wrote: -----
>
>> To: linux-s390@vger.kernel.org
>> From: Alexander Graf <agraf@suse.de>
>> Date: 2016-03-17 20:01
>> Cc: Evgeny Cherkashin/Russia/IBM@IBMRU, linux-kernel@vger.kernel.org,
>> Heiko Carstens <heiko.carstens@de.ibm.com>, Martin Schwidefsky
>> <schwidefsky@de.ibm.com>, Ursula Braun <ubraun@linux.vnet.ibm.com>,
>> ihno@suse.de, mt@suse.com, pwieczorkiewicz@suse.de
>> Subject: [PATCH] qeth: Default to allow promiscuous mode
>>
>> When a qeth device is in bridge role, one of the ports of an adapter
>> can
>> ask for promiscuous mode and get it enabled.
>>
>> The default until now was to not allow user space to enable
>> promiscuous mode
>> without switching sysfs attributes as well though, diverging from
>> usual
>> network device semantics.
>>
>> This patch sets the default to allow promiscuous enablement. That way
>> all
>> existing tools "just work", albeit only one port of an adapter can be
>> in
>> promiscuous mode at a given time.
>>
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> ---
>> drivers/s390/net/qeth_l2_sys.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/s390/net/qeth_l2_sys.c
>> b/drivers/s390/net/qeth_l2_sys.c
>> index 692db49..98c7ac5 100644
>> --- a/drivers/s390/net/qeth_l2_sys.c
>> +++ b/drivers/s390/net/qeth_l2_sys.c
>> @@ -258,6 +258,10 @@ void qeth_l2_setup_bridgeport_attrs(struct
>> qeth_card *card)
>> return;
>> if (!card->options.sbp.supported_funcs)
>> return;
>> +
>> + /* Allow to set promiscuous by default */
>> + card->options.sbp.reflect_promisc = 1;
>> +
>> if (card->options.sbp.role != QETH_SBP_ROLE_NONE) {
>> /* Conditional to avoid spurious error messages */
>> qeth_bridgeport_setrole(card, card->options.sbp.role);
>> --
>> 1.8.5.6
>>
>>
>
> 1. The patch changes the default behaviour (which is, on s390, to ignore promisc setting) and has potential to break existing setups. One potentially dangerous scenario is that a Linux instance may inadvertently snatch the BRIDGEPORT role from a zVM VSWITCH that bridges the HiperSockets LAN to the outer world, disrupting connectivity for many users.
Look at it the other way around. Hardware allows you to do X, why should
Linux prevent you from doing X then?
If you can break another LPAR on the system, then your virtualization
solution sucks and people should be aware of it and simply configure it
in a way that it doesn't. If you can't trust your admin of a Linux
system to call the right commands, you can't trust him to not be
malicious either and break it on purpose.
> 2. The patch sets the reflect_promisc field without setting the reflect_promisc_primary field, making it unclear to the user what is the default role. (It will be "secondary" but it's better to set it explicitly.)
I can send it explicitly if that makes it move obvious, sure.
> 3. The patch should probably go via the -networking maillist.
Ah, get_maintainers.pl didn't list it. You may want to send a patch for
the MAINTAINERS file here :).
> It seems to me that it would be better to use the sysfs attribute from a startup script or a udev rule, *only* when the system configuration requires "real" promiscuous mode, e.g. when the interface is a member of a software bridge.
You've now successfully added a lot of headaches, curses and pain to the
plate of the admin of such a system. As an admin of such a system, I
don't want to have to deal with s390x specifics. That's why I'm using
Linux there.
And who decides what "real" promiscuous mode is? Nobody should set a
network device into promiscuous mode today already unless it's required.
The current thinking in your company seems to be that "real" users
should explicitly set sysfs attribute. Let's think this a bit ahead.
Imagine a world with this sysfs attribute. Every "real" user of
promiscuous mode will now set the sysfs attribute as well in parallel to
setting promiscuous mode on the device. Once you have patched 50 user
space packages to touch something they really shouldn't have to care
about, the same applications that today set an interface to promiscuous
mode, set it to promiscuous mode, just by writing to sysfs plus enabling it.
So at the end of the day, the only thing you've done is you've created a
lot of work for people who are not you, for no gain.
That leaves only one option. We can create a udev rule that *always*
sets the device to promiscuous capable. But that's the same net result
as the patch I've sent, no?
Alex
^ permalink raw reply
* Re: [PATCH net-next] net/mlx4_core: Fix backward compatibility on VFs
From: Alexey Kardashevskiy @ 2016-03-18 9:45 UTC (permalink / raw)
To: Eli Cohen, davem-fT/PcQaiUtIeIZ0/mPfg9Q
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1458233382-12871-1-git-send-email-eli-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
On 03/18/2016 03:49 AM, Eli Cohen wrote:
> Commit 85743f1eb345 ("net/mlx4_core: Set UAR page size to 4KB regardless
> of system page size") introduced dependency where old VF drivers without
> this fix fail to load if the PF driver runs with this commit.
>
> To resolve this add a module parameter which disables that functionality
> by default. If both the PF and VFs are running with a driver with that
> commit the administrator may set the module param to true.
>
> The module parameter is called enable_4k_uar.
>
> Fixes: 85743f1eb345 ('net/mlx4_core: Set UAR page size to 4KB ...')
> Signed-off-by: Eli Cohen <eli-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Thanks!
Tested-by: Alexey Kardashevskiy <aik-sLpHqDYs0B2HXe+LvDLADg@public.gmane.org>
> ---
> drivers/net/ethernet/mellanox/mlx4/main.c | 24 ++++++++++++++++++------
> 1 file changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
> index 503ec23e84cc..358f7230da58 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/main.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/main.c
> @@ -105,6 +105,11 @@ module_param(enable_64b_cqe_eqe, bool, 0444);
> MODULE_PARM_DESC(enable_64b_cqe_eqe,
> "Enable 64 byte CQEs/EQEs when the FW supports this (default: True)");
>
> +static bool enable_4k_uar;
> +module_param(enable_4k_uar, bool, 0444);
> +MODULE_PARM_DESC(enable_4k_uar,
> + "Enable using 4K UAR. Should not be enabled if have VFs which do not support 4K UARs (default: false)");
> +
> #define PF_CONTEXT_BEHAVIOUR_MASK (MLX4_FUNC_CAP_64B_EQE_CQE | \
> MLX4_FUNC_CAP_EQE_CQE_STRIDE | \
> MLX4_FUNC_CAP_DMFS_A0_STATIC)
> @@ -423,7 +428,11 @@ static int mlx4_dev_cap(struct mlx4_dev *dev, struct mlx4_dev_cap *dev_cap)
> /* Virtual PCI function needs to determine UAR page size from
> * firmware. Only master PCI function can set the uar page size
> */
> - dev->uar_page_shift = DEFAULT_UAR_PAGE_SHIFT;
> + if (enable_4k_uar)
> + dev->uar_page_shift = DEFAULT_UAR_PAGE_SHIFT;
> + else
> + dev->uar_page_shift = PAGE_SHIFT;
> +
> mlx4_set_num_reserved_uars(dev, dev_cap);
> }
>
> @@ -2233,11 +2242,14 @@ static int mlx4_init_hca(struct mlx4_dev *dev)
>
> dev->caps.max_fmr_maps = (1 << (32 - ilog2(dev->caps.num_mpts))) - 1;
>
> - /* Always set UAR page size 4KB, set log_uar_sz accordingly */
> - init_hca.log_uar_sz = ilog2(dev->caps.num_uars) +
> - PAGE_SHIFT -
> - DEFAULT_UAR_PAGE_SHIFT;
> - init_hca.uar_page_sz = DEFAULT_UAR_PAGE_SHIFT - 12;
> + if (enable_4k_uar) {
> + init_hca.log_uar_sz = ilog2(dev->caps.num_uars) +
> + PAGE_SHIFT - DEFAULT_UAR_PAGE_SHIFT;
> + init_hca.uar_page_sz = DEFAULT_UAR_PAGE_SHIFT - 12;
> + } else {
> + init_hca.log_uar_sz = ilog2(dev->caps.num_uars);
> + init_hca.uar_page_sz = PAGE_SHIFT - 12;
> + }
>
> init_hca.mw_enabled = 0;
> if (dev->caps.flags & MLX4_DEV_CAP_FLAG_MEM_WINDOW ||
>
--
Alexey
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH net] bonding: fix bond_get_stats()
From: Nikolay Aleksandrov @ 2016-03-18 11:26 UTC (permalink / raw)
To: Eric Dumazet, David Miller
Cc: netdev, Andy Gospodarek, Jay Vosburgh, Veaceslav Falico
In-Reply-To: <1458260616.31963.14.camel@edumazet-glaptop3.roam.corp.google.com>
On 03/18/2016 01:23 AM, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> bond_get_stats() can be called from rtnetlink (with RTNL held)
> or from /proc/net/dev seq handler (with RCU held)
>
> The logic added in commit 5f0c5f73e5ef ("bonding: make global bonding
> stats more reliable") kind of assumed only one cpu could run there.
>
> If multiple threads are reading /proc/net/dev, stats can be really
> messed up after a while.
>
> A second problem is that some fields are 32bit, so we need to properly
> handle the wrap around problem.
>
> Given that RTNL is not always held, we need to use
> bond_for_each_slave_rcu().
>
> Fixes: 5f0c5f73e5ef ("bonding: make global bonding stats more reliable")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Andy Gospodarek <gospo@cumulusnetworks.com>
> Cc: Jay Vosburgh <j.vosburgh@gmail.com>
> Cc: Veaceslav Falico <vfalico@gmail.com>
> ---
> drivers/net/bonding/bond_main.c | 66 +++++++++++++++---------------
> include/net/bonding.h | 1
> 2 files changed, 36 insertions(+), 31 deletions(-)
>
Good catches! And the dev_get_stats() in bond_enslave is fine because the slave
is not yet linked thus can't collide with reading/updating the stats via procfs.
Reviewed-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
^ permalink raw reply
* Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB
From: Mason @ 2016-03-18 12:12 UTC (permalink / raw)
To: Sebastian Frias
Cc: Uwe Kleine-Koenig, David S. Miller, netdev, LKML, Daniel Mack,
Florian Fainelli, Mans Rullgard, Fabio Estevam,
Martin Blumenstingl
In-Reply-To: <56E99727.9040702@laposte.net>
[ CCing a few devs who might be interested ]
On 16/03/2016 18:25, Sebastian Frias wrote:
> Commit 687908c2b649 ("net: phy: at803x: simplify using
> devm_gpiod_get_optional and its 4th argument") introduced a dependency
> on GPIOLIB that was not there before.
>
> This commit removes such dependency by checking the return code and
> comparing it against ENOSYS which is returned when GPIOLIB is not
> selected.
>
> Fixes: 687908c2b649 ("net: phy: at803x: simplify using
> devm_gpiod_get_optional and its 4th argument")
>
> Signed-off-by: Sebastian Frias <sf84@laposte.net>
> ---
> drivers/net/phy/at803x.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
> index 2174ec9..88b7ff3 100644
> --- a/drivers/net/phy/at803x.c
> +++ b/drivers/net/phy/at803x.c
> @@ -252,7 +252,9 @@ static int at803x_probe(struct phy_device *phydev)
> return -ENOMEM;
>
> gpiod_reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
> - if (IS_ERR(gpiod_reset))
> + if (PTR_ERR(gpiod_reset) == -ENOSYS)
> + gpiod_reset = NULL;
> + else if (IS_ERR(gpiod_reset))
> return PTR_ERR(gpiod_reset);
>
> priv->gpiod_reset = gpiod_reset;
>
^ permalink raw reply
* [PATCH ethtool] ethtool.c: fix memory leaks
From: Ivan Vecera @ 2016-03-18 12:24 UTC (permalink / raw)
To: netdev; +Cc: bwh
Memory allocated at several places is not appropriately freed.
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
ethtool.c | 60 +++++++++++++++++++++++++++++++++++++++++++++---------------
1 file changed, 45 insertions(+), 15 deletions(-)
diff --git a/ethtool.c b/ethtool.c
index 0cd0d4f..ca0bf28 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -2065,10 +2065,14 @@ static int do_gfeatures(struct cmd_context *ctx)
features = get_features(ctx, defs);
if (!features) {
fprintf(stdout, "no feature info available\n");
+ free(defs);
return 1;
}
dump_features(defs, features, NULL);
+
+ free(features);
+ free(defs);
return 0;
}
@@ -2078,11 +2082,11 @@ static int do_sfeatures(struct cmd_context *ctx)
int any_changed = 0, any_mismatch = 0;
u32 off_flags_wanted = 0;
u32 off_flags_mask = 0;
- struct ethtool_sfeatures *efeatures;
+ struct ethtool_sfeatures *efeatures = NULL;
struct cmdline_info *cmdline_features;
- struct feature_state *old_state, *new_state;
+ struct feature_state *old_state = NULL, *new_state = NULL;
struct ethtool_value eval;
- int err;
+ int err, retval = 1;
int i, j;
defs = get_feature_defs(ctx);
@@ -2096,7 +2100,7 @@ static int do_sfeatures(struct cmd_context *ctx)
sizeof(efeatures->features[0]));
if (!efeatures) {
perror("Cannot parse arguments");
- return 1;
+ goto finish;
}
efeatures->cmd = ETHTOOL_SFEATURES;
efeatures->size = FEATURE_BITS_TO_BLOCKS(defs->n_features);
@@ -2114,7 +2118,7 @@ static int do_sfeatures(struct cmd_context *ctx)
sizeof(cmdline_features[0]));
if (!cmdline_features) {
perror("Cannot parse arguments");
- return 1;
+ goto finish;
}
for (i = 0; i < ARRAY_SIZE(off_flag_def); i++)
flag_to_cmdline_info(off_flag_def[i].short_name,
@@ -2133,12 +2137,13 @@ static int do_sfeatures(struct cmd_context *ctx)
if (!any_changed) {
fprintf(stdout, "no features changed\n");
- return 0;
+ retval = 0;
+ goto finish;
}
old_state = get_features(ctx, defs);
if (!old_state)
- return 1;
+ goto finish;
if (efeatures) {
/* For each offload that the user specified, update any
@@ -2182,7 +2187,7 @@ static int do_sfeatures(struct cmd_context *ctx)
err = send_ioctl(ctx, efeatures);
if (err < 0) {
perror("Cannot set device feature settings");
- return 1;
+ goto finish;
}
} else {
for (i = 0; i < ARRAY_SIZE(off_flag_def); i++) {
@@ -2197,7 +2202,7 @@ static int do_sfeatures(struct cmd_context *ctx)
fprintf(stderr,
"Cannot set device %s settings: %m\n",
off_flag_def[i].long_name);
- return 1;
+ goto finish;
}
}
}
@@ -2211,7 +2216,8 @@ static int do_sfeatures(struct cmd_context *ctx)
err = send_ioctl(ctx, &eval);
if (err) {
perror("Cannot set device flag settings");
- return 92;
+ retval = 92;
+ goto finish;
}
}
}
@@ -2219,7 +2225,7 @@ static int do_sfeatures(struct cmd_context *ctx)
/* Compare new state with requested state */
new_state = get_features(ctx, defs);
if (!new_state)
- return 1;
+ goto finish;
any_changed = new_state->off_flags != old_state->off_flags;
any_mismatch = (new_state->off_flags !=
((old_state->off_flags & ~off_flags_mask) |
@@ -2238,13 +2244,19 @@ static int do_sfeatures(struct cmd_context *ctx)
if (!any_changed) {
fprintf(stderr,
"Could not change any device features\n");
- return 1;
+ goto finish;
}
printf("Actual changes:\n");
dump_features(defs, new_state, old_state);
}
- return 0;
+ retval = 0;
+finish:
+ free(new_state);
+ free(old_state);
+ free(efeatures);
+ free(defs);
+ return retval;
}
static int do_gset(struct cmd_context *ctx)
@@ -2705,8 +2717,18 @@ static int do_gregs(struct cmd_context *ctx)
return 75;
}
- regs = realloc(regs, sizeof(*regs) + st.st_size);
- regs->len = st.st_size;
+ if (regs->len != st.st_size) {
+ struct ethtool_regs *new_regs;
+ new_regs = realloc(regs, sizeof(*regs) + st.st_size);
+ if (!new_regs) {
+ perror("Cannot allocate memory for register "
+ "dump");
+ free(regs);
+ return 73;
+ }
+ regs = new_regs;
+ regs->len = st.st_size;
+ }
nread = fread(regs->data, regs->len, 1, f);
fclose(f);
if (nread != 1) {
@@ -3775,6 +3797,7 @@ static int do_gprivflags(struct cmd_context *ctx)
}
if (strings->len == 0) {
fprintf(stderr, "No private flags defined\n");
+ free(strings);
return 1;
}
if (strings->len > 32) {
@@ -3786,6 +3809,7 @@ static int do_gprivflags(struct cmd_context *ctx)
flags.cmd = ETHTOOL_GPFLAGS;
if (send_ioctl(ctx, &flags)) {
perror("Cannot get private flags");
+ free(strings);
return 1;
}
@@ -3804,6 +3828,7 @@ static int do_gprivflags(struct cmd_context *ctx)
(const char *)strings->data + i * ETH_GSTRING_LEN,
(flags.data & (1U << i)) ? "on" : "off");
+ free(strings);
return 0;
}
@@ -3825,6 +3850,7 @@ static int do_sprivflags(struct cmd_context *ctx)
}
if (strings->len == 0) {
fprintf(stderr, "No private flags defined\n");
+ free(strings);
return 1;
}
if (strings->len > 32) {
@@ -3836,6 +3862,7 @@ static int do_sprivflags(struct cmd_context *ctx)
cmdline = calloc(strings->len, sizeof(*cmdline));
if (!cmdline) {
perror("Cannot parse arguments");
+ free(strings);
return 1;
}
for (i = 0; i < strings->len; i++) {
@@ -3852,6 +3879,7 @@ static int do_sprivflags(struct cmd_context *ctx)
flags.cmd = ETHTOOL_GPFLAGS;
if (send_ioctl(ctx, &flags)) {
perror("Cannot get private flags");
+ free(strings);
return 1;
}
@@ -3859,9 +3887,11 @@ static int do_sprivflags(struct cmd_context *ctx)
flags.data = (flags.data & ~seen_flags) | wanted_flags;
if (send_ioctl(ctx, &flags)) {
perror("Cannot set private flags");
+ free(strings);
return 1;
}
+ free(strings);
return 0;
}
--
2.7.3
^ permalink raw reply related
* Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB
From: Uwe Kleine-König @ 2016-03-18 12:54 UTC (permalink / raw)
To: Sebastian Frias
Cc: David S. Miller, netdev, lkml, mason, Daniel Mack,
Florian Fainelli, Mans Rullgard, Fabio Estevam,
Martin Blumenstingl, Linus Walleij
In-Reply-To: <56E99727.9040702@laposte.net>
[expand cc a bit more]
Hello,
On Wed, Mar 16, 2016 at 06:25:59PM +0100, Sebastian Frias wrote:
> Commit 687908c2b649 ("net: phy: at803x: simplify using
> devm_gpiod_get_optional and its 4th argument") introduced a dependency
> on GPIOLIB that was not there before.
>
> This commit removes such dependency by checking the return code and
> comparing it against ENOSYS which is returned when GPIOLIB is not
> selected.
>
> Fixes: 687908c2b649 ("net: phy: at803x: simplify using devm_gpiod_get_optional and its 4th argument")
>
> Signed-off-by: Sebastian Frias <sf84@laposte.net>
> ---
> drivers/net/phy/at803x.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
> index 2174ec9..88b7ff3 100644
> --- a/drivers/net/phy/at803x.c
> +++ b/drivers/net/phy/at803x.c
> @@ -252,7 +252,9 @@ static int at803x_probe(struct phy_device *phydev)
> return -ENOMEM;
>
> gpiod_reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
> - if (IS_ERR(gpiod_reset))
> + if (PTR_ERR(gpiod_reset) == -ENOSYS)
> + gpiod_reset = NULL;
> + else if (IS_ERR(gpiod_reset))
this isn't better either (IMHO it's worse, but maybe this is debatable
and also depends on your POV).
With 687908c2b649 I made kernels without GPIOLIB fail to bind this
device. I admit this is not maximally nice.
Your change makes the driver bind in this case again and then the reset
gpio isn't handled at all which might result in a later and harder to
debug error.
The better approach to fix your problem is: make the driver depend (or
force) on GPIOLIB. Or alternatively, drop reset-handling from the
driver.
From a driver perspecitive, it would be nice if devm_gpiod_get_optional
returned NULL iff the respective gpio isn't specified even with
GPIOLIB=n, but this isn't sensible either because it would result in
quite some gpiolib code to not being conditionally compiled on
CONFIG_GPIOLIB any more.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* Re: [PATCH] openvswitch: call only into reachable nf-nat code
From: Arnd Bergmann @ 2016-03-18 12:57 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: dev-yBygre7rU0TnMu66kgdUjQ, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Joe Stringer, Paolo Abeni,
David S. Miller
In-Reply-To: <1458132481-318209-1-git-send-email-arnd-r2nGTMty4D4@public.gmane.org>
On Wednesday 16 March 2016 13:47:13 Arnd Bergmann wrote:
> The openvswitch code has gained support for calling into the
> nf-nat-ipv4/ipv6 modules, however those can be loadable modules
> in a configuration in which openvswitch is built-in, leading
> to link errors:
>
> net/built-in.o: In function `__ovs_ct_lookup':
> :(.text+0x2cc2c8): undefined reference to `nf_nat_icmp_reply_translation'
> :(.text+0x2cc66c): undefined reference to `nf_nat_icmpv6_reply_translation'
>
> The dependency on (!NF_NAT || NF_NAT) was meant to prevent
> this, but NF_NAT is set to 'y' if any of the symbols selecting
> it are built-in, but the link error happens when any of them
> are modular.
>
> A second issue is that even if CONFIG_NF_NAT_IPV6 is built-in,
> CONFIG_NF_NAT_IPV4 might be completely disabled. This is unlikely
> to be useful in practice, but the driver currently only handles
> IPv6 being optional.
>
> This patch improves the Kconfig dependency so that openvswitch
> cannot be built-in if either of the two other symbols are set
> to 'm', and it replaces the incorrect #ifdef in ovs_ct_nat_execute()
> with two "if (IS_ENABLED())" checks that should catch all corner
> cases also make the code more readable.
>
> The same #ifdef exists ovs_ct_nat_to_attr(), where it does not
> cause a link error, but for consistency I'm changing it the same
> way.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: 05752523e565 ("openvswitch: Interface with NAT.")
> ---
> net/openvswitch/Kconfig | 3 ++-
> net/openvswitch/conntrack.c | 16 ++++++++--------
> 2 files changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/net/openvswitch/Kconfig b/net/openvswitch/Kconfig
> index 234a73344c6e..961fb60115df 100644
> --- a/net/openvswitch/Kconfig
> +++ b/net/openvswitch/Kconfig
> @@ -7,7 +7,8 @@ config OPENVSWITCH
> depends on INET
> depends on !NF_CONNTRACK || \
> (NF_CONNTRACK && ((!NF_DEFRAG_IPV6 || NF_DEFRAG_IPV6) && \
> - (!NF_NAT || NF_NAT)))
> + (!NF_NAT_IPV4 || NF_NAT_IPV4) && \
> + (!NF_NAT_IPV6 || NF_NAT_IPV6)))
> select LIBCRC32C
> select MPLS
> select NET_MPLS_GSO
I've now seen a new regression:
net/built-in.o: In function `__ovs_ct_lookup':
switchdev.c:(.text+0x136e8c): undefined reference to `nf_ct_nat_ext_add'
switchdev.c:(.text+0x136f38): undefined reference to `nf_nat_packet'
switchdev.c:(.text+0x136f80): undefined reference to `nf_nat_setup_info'
switchdev.c:(.text+0x136f98): undefined reference to `nf_nat_alloc_null_binding'
Apparently, the (!NF_NAT || NF_NAT) statement is also needed in addition to
the other two. I'm resending the fixed patch, as it doesn't seem to have
been merged yet.
If it's in some other tree already and you'd rather have a patch on top,
I can send that too.
Arnd
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
^ permalink raw reply
* Re: [PATCH] qeth: Default to allow promiscuous mode
From: Evgeny Cherkashin @ 2016-03-18 13:07 UTC (permalink / raw)
To: Alexander Graf
Cc: linux-s390, linux-kernel, heicars2, mschwid2, Ursula Braun, ihno,
mt, pwieczorkiewicz, netdev@vger.kernel.org
In-Reply-To: <56EBCC28.5090500@suse.de>
Hello Alexander,
-----Alexander Graf <agraf@suse.de> wrote: -----
send it explicitly if that makes it move obvious, sure.
>
>> 3. The patch should probably go via the -networking maillist.
>
>Ah, get_maintainers.pl didn't list it. You may want to send a patch
>for
>the MAINTAINERS file here :).
My mistake, you've sent it to the right place.
About the nature of the patch:
- Enabling bridgeport on adaptors on which bridgeport is already used by somebody else will break somebody else's operation.
- Therefore, system administrator must only enable bridgeport on adaptors on which nobody else tries to use it (without coordinating with them).
- Making bridge_reflect_promisc enabled by default makes it impossible.
- Therefore, bridge_reflect_promisc must not be enabled by default.
(Those who enable bridge_reflect_promisc blindly on all adaptors will break their users' setup, and will have to deal with the consequences.)
Regards,
Evgeny Cherkashin / Евгений Черкашин / Eugene Crosser (preferred)
Software Engineer, IBM Science and Technology Center, Linux on z-Systems Development
tel. +7 495 775 8800 ext.1103, Moscow 123317, Presnenskaya emb. 10
^ permalink raw reply
* Re: [PATCH] qeth: Default to allow promiscuous mode
From: Evgeny Cherkashin @ 2016-03-18 13:07 UTC (permalink / raw)
To: Alexander Graf
Cc: linux-s390, linux-kernel, heicars2, mschwid2, Ursula Braun, ihno,
mt, pwieczorkiewicz, netdev@vger.kernel.org
In-Reply-To: <56EBCC28.5090500@suse.de>
Hello Alexander,
-----Alexander Graf <agraf@suse.de> wrote: -----
send it explicitly if that makes it move obvious, sure.
>
>> 3. The patch should probably go via the -networking maillist.
>
>Ah, get_maintainers.pl didn't list it. You may want to send a patch
>for
>the MAINTAINERS file here :).
My mistake, you've sent it to the right place.
About the nature of the patch:
- Enabling bridgeport on adaptors on which bridgeport is already used by somebody else will break somebody else's operation.
- Therefore, system administrator must only enable bridgeport on adaptors on which nobody else tries to use it (without coordinating with them).
- Making bridge_reflect_promisc enabled by default makes it impossible.
- Therefore, bridge_reflect_promisc must not be enabled by default.
(Those who enable bridge_reflect_promisc blindly on all adaptors will break their users' setup, and will have to deal with the consequences.)
Regards,
Evgeny Cherkashin / Евгений Черкашин / Eugene Crosser (preferred)
Software Engineer, IBM Science and Technology Center, Linux on z-Systems Development
tel. +7 495 775 8800 ext.1103, Moscow 123317, Presnenskaya emb. 10
^ permalink raw reply
* [PATCH v2] openvswitch: call only into reachable nf-nat code
From: Arnd Bergmann @ 2016-03-18 13:33 UTC (permalink / raw)
To: Pravin Shelar, David S. Miller
Cc: dev-yBygre7rU0TnMu66kgdUjQ, Arnd Bergmann,
netdev-u79uwXL29TY76Z2rM5mHXA, Florian Westphal,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Joe Stringer,
Eric W. Biederman, Paolo Abeni, Pablo Neira Ayuso
The openvswitch code has gained support for calling into the
nf-nat-ipv4/ipv6 modules, however those can be loadable modules
in a configuration in which openvswitch is built-in, leading
to link errors:
net/built-in.o: In function `__ovs_ct_lookup':
:(.text+0x2cc2c8): undefined reference to `nf_nat_icmp_reply_translation'
:(.text+0x2cc66c): undefined reference to `nf_nat_icmpv6_reply_translation'
The dependency on (!NF_NAT || NF_NAT) prevents similar issues,
but NF_NAT is set to 'y' if any of the symbols selecting
it are built-in, but the link error happens when any of them
are modular.
A second issue is that even if CONFIG_NF_NAT_IPV6 is built-in,
CONFIG_NF_NAT_IPV4 might be completely disabled. This is unlikely
to be useful in practice, but the driver currently only handles
IPv6 being optional.
This patch improves the Kconfig dependency so that openvswitch
cannot be built-in if either of the two other symbols are set
to 'm', and it replaces the incorrect #ifdef in ovs_ct_nat_execute()
with two "if (IS_ENABLED())" checks that should catch all corner
cases also make the code more readable.
The same #ifdef exists ovs_ct_nat_to_attr(), where it does not
cause a link error, but for consistency I'm changing it the same
way.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: 05752523e565 ("openvswitch: Interface with NAT.")
Acked-by: Joe Stringer <joe@ovn.org>
---
v2: leave (!NF_NAT || NF_NAT) dependency in there, we also need that
net/openvswitch/Kconfig | 4 +++-
net/openvswitch/conntrack.c | 16 ++++++++--------
2 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/net/openvswitch/Kconfig b/net/openvswitch/Kconfig
index 234a73344c6e..ce947292ae77 100644
--- a/net/openvswitch/Kconfig
+++ b/net/openvswitch/Kconfig
@@ -7,7 +7,9 @@ config OPENVSWITCH
depends on INET
depends on !NF_CONNTRACK || \
(NF_CONNTRACK && ((!NF_DEFRAG_IPV6 || NF_DEFRAG_IPV6) && \
- (!NF_NAT || NF_NAT)))
+ (!NF_NAT || NF_NAT) && \
+ (!NF_NAT_IPV4 || NF_NAT_IPV4) && \
+ (!NF_NAT_IPV6 || NF_NAT_IPV6)))
select LIBCRC32C
select MPLS
select NET_MPLS_GSO
diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index dc5eb29fe7d6..ff04b5db04b3 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -535,14 +535,15 @@ static int ovs_ct_nat_execute(struct sk_buff *skb, struct nf_conn *ct,
switch (ctinfo) {
case IP_CT_RELATED:
case IP_CT_RELATED_REPLY:
- if (skb->protocol == htons(ETH_P_IP) &&
+ if (IS_ENABLED(CONFIG_NF_NAT_IPV4) &&
+ skb->protocol == htons(ETH_P_IP) &&
ip_hdr(skb)->protocol == IPPROTO_ICMP) {
if (!nf_nat_icmp_reply_translation(skb, ct, ctinfo,
hooknum))
err = NF_DROP;
goto push;
-#if IS_ENABLED(CONFIG_NF_NAT_IPV6)
- } else if (skb->protocol == htons(ETH_P_IPV6)) {
+ } else if (IS_ENABLED(CONFIG_NF_NAT_IPV6) &&
+ skb->protocol == htons(ETH_P_IPV6)) {
__be16 frag_off;
u8 nexthdr = ipv6_hdr(skb)->nexthdr;
int hdrlen = ipv6_skip_exthdr(skb,
@@ -557,7 +558,6 @@ static int ovs_ct_nat_execute(struct sk_buff *skb, struct nf_conn *ct,
err = NF_DROP;
goto push;
}
-#endif
}
/* Non-ICMP, fall thru to initialize if needed. */
case IP_CT_NEW:
@@ -1238,7 +1238,8 @@ static bool ovs_ct_nat_to_attr(const struct ovs_conntrack_info *info,
}
if (info->range.flags & NF_NAT_RANGE_MAP_IPS) {
- if (info->family == NFPROTO_IPV4) {
+ if (IS_ENABLED(CONFIG_NF_NAT_IPV4) &&
+ info->family == NFPROTO_IPV4) {
if (nla_put_in_addr(skb, OVS_NAT_ATTR_IP_MIN,
info->range.min_addr.ip) ||
(info->range.max_addr.ip
@@ -1246,8 +1247,8 @@ static bool ovs_ct_nat_to_attr(const struct ovs_conntrack_info *info,
(nla_put_in_addr(skb, OVS_NAT_ATTR_IP_MAX,
info->range.max_addr.ip))))
return false;
-#if IS_ENABLED(CONFIG_NF_NAT_IPV6)
- } else if (info->family == NFPROTO_IPV6) {
+ } else if (IS_ENABLED(CONFIG_NF_NAT_IPV6) &&
+ info->family == NFPROTO_IPV6) {
if (nla_put_in6_addr(skb, OVS_NAT_ATTR_IP_MIN,
&info->range.min_addr.in6) ||
(memcmp(&info->range.max_addr.in6,
@@ -1256,7 +1257,6 @@ static bool ovs_ct_nat_to_attr(const struct ovs_conntrack_info *info,
(nla_put_in6_addr(skb, OVS_NAT_ATTR_IP_MAX,
&info->range.max_addr.in6))))
return false;
-#endif
} else {
return false;
}
--
2.7.0
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
^ permalink raw reply related
* [PATCH] openvswitch: reduce padding in struct sw_flow_key
From: Arnd Bergmann @ 2016-03-18 13:34 UTC (permalink / raw)
To: Pravin Shelar, David S. Miller
Cc: dev-yBygre7rU0TnMu66kgdUjQ, Arnd Bergmann,
netdev-u79uwXL29TY76Z2rM5mHXA, Jiri Benc,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Joe Stringer, Justin Pettit
It's been a while since the last time sw_flow_key was made smaller in
1139e241ec43 ("openvswitch: Compact sw_flow_key."), and it has seen five
patches adding new members since then.
With the current linux-next kernel and gcc-6.0 on ARM, this has tipped
us slightly over the stack frame warning limit of 1024 bytes:
net/openvswitch/datapath.c: In function 'ovs_flow_cmd_set':
net/openvswitch/datapath.c:1202:1: error: the frame size of 1032 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
This slightly rearranges the members in struct sw_flow_key to minimize
the amount of padding we have between them, bringing us again slightly
below the warning limit (checking all files against 128 bytes limit):
datapath.c: In function 'get_flow_actions.constprop':
datapath.c:1083:1: warning: the frame size of 464 bytes is larger than 128 bytes
datapath.c: In function 'ovs_flow_cmd_new':
datapath.c:1061:1: warning: the frame size of 984 bytes is larger than 128 bytes
datapath.c: In function 'ovs_flow_cmd_del':
datapath.c:1336:1: warning: the frame size of 528 bytes is larger than 128 bytes
datapath.c: In function 'ovs_flow_cmd_get':
datapath.c:1261:1: warning: the frame size of 504 bytes is larger than 128 bytes
datapath.c: In function 'ovs_flow_cmd_set':
datapath.c:1202:1: warning: the frame size of 1016 bytes is larger than 128 bytes
datapath.c: In function 'queue_gso_packets':
datapath.c:379:1: warning: the frame size of 472 bytes is larger than 128 bytes
flow_table.c: In function 'masked_flow_lookup':
flow_table.c:489:1: warning: the frame size of 488 bytes is larger than 128 bytes
flow_netlink.c: In function 'validate_and_copy_set_tun':
flow_netlink.c:1994:1: warning: the frame size of 512 bytes is larger than 128 bytes
This means it's still too large really, we just don't warn about it any more,
and will get the warning again once another member is added. My patch is a
band-aid at best, but more work is needed here. One problem is that
ovs_flow_cmd_new and ovs_flow_cmd_set have two copies of struct sw_flow_key on
the stack, one of them nested within sw_flow_mask. If we could reduce that to
a single instance, the stack usage would be cut in half here.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: 00a93babd06a ("openvswitch: add tunnel protocol to sw_flow_key")
Fixes: c2ac66735870 ("openvswitch: Allow matching on conntrack label")
Fixes: 182e3042e15d ("openvswitch: Allow matching on conntrack mark")
Fixes: 7f8a436eaa2c ("openvswitch: Add conntrack action")
Fixes: 971427f353f3 ("openvswitch: Add recirc and hash action.")
---
net/openvswitch/flow.h | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/net/openvswitch/flow.h b/net/openvswitch/flow.h
index 1d055c559eaf..41d15c50a43f 100644
--- a/net/openvswitch/flow.h
+++ b/net/openvswitch/flow.h
@@ -63,7 +63,11 @@ struct sw_flow_key {
u32 skb_mark; /* SKB mark. */
u16 in_port; /* Input switch port (or DP_MAX_PORTS). */
} __packed phy; /* Safe when right after 'tun_key'. */
- u8 tun_proto; /* Protocol of encapsulating tunnel. */
+ struct {
+ __be16 src; /* TCP/UDP/SCTP source port. */
+ __be16 dst; /* TCP/UDP/SCTP destination port. */
+ __be16 flags; /* TCP flags. */
+ } tp;
u32 ovs_flow_hash; /* Datapath computed hash value. */
u32 recirc_id; /* Recirculation ID. */
struct {
@@ -83,11 +87,6 @@ struct sw_flow_key {
u8 frag; /* One of OVS_FRAG_TYPE_*. */
} ip;
};
- struct {
- __be16 src; /* TCP/UDP/SCTP source port. */
- __be16 dst; /* TCP/UDP/SCTP destination port. */
- __be16 flags; /* TCP flags. */
- } tp;
union {
struct {
struct {
@@ -114,11 +113,12 @@ struct sw_flow_key {
};
struct {
/* Connection tracking fields. */
- u16 zone;
u32 mark;
+ u16 zone;
u8 state;
struct ovs_key_ct_labels labels;
} ct;
+ u8 tun_proto; /* Protocol of encapsulating tunnel. */
} __aligned(BITS_PER_LONG/8); /* Ensure that we can do comparisons as longs. */
--
2.7.0
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
^ permalink raw reply related
* Re: [PATCH (net-next.git)] stmmac: fix TX normal DESC
From: Tomeu Vizoso @ 2016-03-18 14:07 UTC (permalink / raw)
To: Giuseppe Cavallaro
Cc: netdev, Gabriel Fernandez, Andreas Färber,
Frank Schäfer, Dinh Nguyen, David S. Miller, preid,
Fabrice GASNIER, Alexandre TORGUE
In-Reply-To: <1458123709-19692-2-git-send-email-peppe.cavallaro@st.com>
On 16 March 2016 at 11:21, Giuseppe Cavallaro <peppe.cavallaro@st.com> wrote:
> This patch fixs a regression raised when test on chips that use
> the normal descriptor layout. In fact, no len bits were set for
> the TDES1 and no OWN bit inside the TDES0.
>
> Signed-off-by: Giuseppe CAVALLARO <peppe.cavallaro@st.com>
> Tested-by: Andreas Färber <afaerber@suse.de>
> Cc: Fabrice Gasnier <fabrice.gasnier@st.com>
Hi,
with this patch, I have eth working reliably on the Rock2 again.
Tested-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
Thanks,
Tomeu
> ---
> drivers/net/ethernet/stmicro/stmmac/norm_desc.c | 16 ++++++++--------
> 1 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/norm_desc.c b/drivers/net/ethernet/stmicro/stmmac/norm_desc.c
> index e13228f..011386f 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/norm_desc.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/norm_desc.c
> @@ -199,11 +199,6 @@ static void ndesc_prepare_tx_desc(struct dma_desc *p, int is_fs, int len,
> {
> unsigned int tdes1 = p->des1;
>
> - if (mode == STMMAC_CHAIN_MODE)
> - norm_set_tx_desc_len_on_chain(p, len);
> - else
> - norm_set_tx_desc_len_on_ring(p, len);
> -
> if (is_fs)
> tdes1 |= TDES1_FIRST_SEGMENT;
> else
> @@ -217,10 +212,15 @@ static void ndesc_prepare_tx_desc(struct dma_desc *p, int is_fs, int len,
> if (ls)
> tdes1 |= TDES1_LAST_SEGMENT;
>
> - if (tx_own)
> - tdes1 |= TDES0_OWN;
> -
> p->des1 = tdes1;
> +
> + if (mode == STMMAC_CHAIN_MODE)
> + norm_set_tx_desc_len_on_chain(p, len);
> + else
> + norm_set_tx_desc_len_on_ring(p, len);
> +
> + if (tx_own)
> + p->des0 |= TDES0_OWN;
> }
>
> static void ndesc_set_tx_ic(struct dma_desc *p)
> --
> 1.7.4.4
>
^ permalink raw reply
* Re: [PATCH v6 net-next 0/2] tcp: Redundant Data Bundling (RDB)
From: Bendik Rønning Opstad @ 2016-03-18 14:25 UTC (permalink / raw)
To: Yuchung Cheng, Bendik Rønning Opstad
Cc: David S. Miller, netdev, Eric Dumazet, Neal Cardwell,
Andreas Petlund, Carsten Griwodz, Pål Halvorsen,
Jonas Markussen, Kristian Evensen, Kenneth Klette Jonassen
In-Reply-To: <CAK6E8=e5tVX3cfwvaXxN7LEq6XggeT5Y7T0j4KcKRsA1b-77mQ@mail.gmail.com>
On 14/03/16 22:59, Yuchung Cheng wrote:
> OK that makes sense.
>
> I left some detailed comments on the actual patches. I would encourage
> to submit an IETF draft to gather feedback from tcpm b/c the feature
> seems portable.
Thank you for the suggestion, we appreciate the confidence. We have
had in mind to eventually pursue a standardization process, but have
been unsure about how a mechanism that actively introduces redundancy
would be received by the IETF. It may now be the right time to propose
the RDB mechanism, and we will aim to present an IEFT draft in the
near future.
Bendik
^ permalink raw reply
* [linux-next:master 4720/12950] net/core/dst_cache.c:31:6: sparse: symbol 'dst_cache_per_cpu_dst_set' was not declared. Should it be static?
From: kbuild test robot @ 2016-03-18 15:27 UTC (permalink / raw)
To: Paolo Abeni; +Cc: kbuild-all, netdev, linux-kernel
tree: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
head: 5e3497cca281616e7930b74a0076b7324dcc2057
commit: 607f725f6f7d5ec3759fbc16224afb60e2152a5b [4720/12950] net: replace dst_cache ip6_tunnel implementation with the generic one
reproduce:
# apt-get install sparse
git checkout 607f725f6f7d5ec3759fbc16224afb60e2152a5b
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__
sparse warnings: (new ones prefixed by >>)
>> net/core/dst_cache.c:31:6: sparse: symbol 'dst_cache_per_cpu_dst_set' was not declared. Should it be static?
>> net/core/dst_cache.c:42:18: sparse: symbol 'dst_cache_per_cpu_get' was not declared. Should it be static?
Please review and possibly fold the followup patch.
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply
* [RFC PATCH linux-next] net: dst_cache_per_cpu_dst_set() can be static
From: kbuild test robot @ 2016-03-18 15:27 UTC (permalink / raw)
To: Paolo Abeni; +Cc: kbuild-all, netdev, linux-kernel
In-Reply-To: <201603182348.v699gI0M%fengguang.wu@intel.com>
Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---
dst_cache.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/core/dst_cache.c b/net/core/dst_cache.c
index 3938f3f..554d364 100644
--- a/net/core/dst_cache.c
+++ b/net/core/dst_cache.c
@@ -28,8 +28,8 @@ struct dst_cache_pcpu {
};
};
-void dst_cache_per_cpu_dst_set(struct dst_cache_pcpu *dst_cache,
- struct dst_entry *dst, u32 cookie)
+static void dst_cache_per_cpu_dst_set(struct dst_cache_pcpu *dst_cache,
+ struct dst_entry *dst, u32 cookie)
{
dst_release(dst_cache->dst);
if (dst)
@@ -39,8 +39,8 @@ void dst_cache_per_cpu_dst_set(struct dst_cache_pcpu *dst_cache,
dst_cache->dst = dst;
}
-struct dst_entry *dst_cache_per_cpu_get(struct dst_cache *dst_cache,
- struct dst_cache_pcpu *idst)
+static struct dst_entry *dst_cache_per_cpu_get(struct dst_cache *dst_cache,
+ struct dst_cache_pcpu *idst)
{
struct dst_entry *dst;
^ permalink raw reply related
* [PATCH v5 0/4] Add Ethernet support on STM32F429
From: Alexandre TORGUE @ 2016-03-18 15:37 UTC (permalink / raw)
To: Maxime Coquelin, Giuseppe Cavallaro, netdev
Cc: linux-arm-kernel, linux-kernel, devicetree, robh, manabian
STM32F429 Chip embeds a Synopsys 3.50a MAC IP.
This series:
-enhance current stmmac driver to control it (code already
available) and adds basic glue for STM32F429 chip.
-Enable basic Net config in kernel.
Note that DT patches are not present because STM32 pinctrl code is not
yet avalaible.
Changes since v4:
-Fix dirty copy/past in bindings documentation patch.
Changes since v3:
-Fix "tx-clk" and "rx-clk" as required clocks. Driver and bindings are
modified.
Changes since v2:
-Fix alphabetic order in Kconfig and Makefile.
-Improve code according to Joachim review.
-Binding: remove useless entry.
Changes since v1:
-Fix Kbuild issue in Kconfig.
-Remove init/exit callbacks. Suspend/Resume and remove driver is no more
driven in stmmac_pltfr but directly in dwmac-stm32 glue driver.
-Take into account Joachim review.
Regards.
Alexandre.
Alexandre TORGUE (4):
net: ethernet: dwmac: add Ethernet glue logic for stm32 chip
Documentation: Bindings: Add STM32 DWMAC glue
net: ethernet: stmmac: add support of Synopsys 3.50a MAC IP
ARM: STM32: Enable Ethernet in stm32_defconfig
.../devicetree/bindings/net/stm32-dwmac.txt | 32 ++++
arch/arm/configs/stm32_defconfig | 9 +
drivers/net/ethernet/stmicro/stmmac/Kconfig | 12 ++
drivers/net/ethernet/stmicro/stmmac/Makefile | 1 +
drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c | 193 +++++++++++++++++++++
.../net/ethernet/stmicro/stmmac/stmmac_platform.c | 1 +
6 files changed, 248 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/stm32-dwmac.txt
create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
--
1.9.1
^ permalink raw reply
* [PATCH v5 2/4] Documentation: Bindings: Add STM32 DWMAC glue
From: Alexandre TORGUE @ 2016-03-18 15:37 UTC (permalink / raw)
To: Maxime Coquelin, Giuseppe Cavallaro, netdev
Cc: linux-arm-kernel, linux-kernel, devicetree, robh, manabian
In-Reply-To: <1458315428-10081-1-git-send-email-alexandre.torgue@gmail.com>
Signed-off-by: Alexandre TORGUE <alexandre.torgue@gmail.com>
diff --git a/Documentation/devicetree/bindings/net/stm32-dwmac.txt b/Documentation/devicetree/bindings/net/stm32-dwmac.txt
new file mode 100644
index 0000000..ada2aa4
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/stm32-dwmac.txt
@@ -0,0 +1,32 @@
+STMicroelectronics STM32 / MCU DWMAC glue layer controller
+
+This file documents platform glue layer for stmmac.
+Please see stmmac.txt for the other unchanged properties.
+
+The device node has following properties.
+
+Required properties:
+- compatible: Should be "st,stm32-dwmac" to select glue, and
+ "snps,dwmac-3.50a" to select IP vesrion.
+- clocks: Must contain a phandle for each entry in clock-names.
+- clock-names: Should be "stmmaceth" for the host clock.
+ Should be "tx-clk" for the MAC TX clock.
+ Should be "rx-clk" for the MAC RX clock.
+- st,syscon : Should be phandle/offset pair. The phandle to the syscon node which
+ encompases the glue register, and the offset of the control register.
+Example:
+
+ ethernet0: dwmac@40028000 {
+ compatible = "st,stm32-dwmac", "snps,dwmac-3.50a";
+ status = "disabled";
+ reg = <0x40028000 0x8000>;
+ reg-names = "stmmaceth";
+ interrupts = <0 61 0>, <0 62 0>;
+ interrupt-names = "macirq", "eth_wake_irq";
+ clock-names = "stmmaceth", "tx-clk", "rx-clk";
+ clocks = <&rcc 0 25>, <&rcc 0 26>, <&rcc 0 27>;
+ st,syscon = <&syscfg 0x4>;
+ snps,pbl = <8>;
+ snps,mixed-burst;
+ dma-ranges;
+ };
--
1.9.1
^ permalink raw reply related
* [PATCH v5 4/4] ARM: STM32: Enable Ethernet in stm32_defconfig
From: Alexandre TORGUE @ 2016-03-18 15:37 UTC (permalink / raw)
To: Maxime Coquelin, Giuseppe Cavallaro, netdev
Cc: linux-arm-kernel, linux-kernel, devicetree, robh, manabian
In-Reply-To: <1458315428-10081-1-git-send-email-alexandre.torgue@gmail.com>
Enable basic Ethernet support (IPV4) for stm32 defconfig.
Signed-off-by: Alexandre TORGUE <alexandre.torgue@gmail.com>
diff --git a/arch/arm/configs/stm32_defconfig b/arch/arm/configs/stm32_defconfig
index ec52505..8b8abe0 100644
--- a/arch/arm/configs/stm32_defconfig
+++ b/arch/arm/configs/stm32_defconfig
@@ -33,11 +33,20 @@ CONFIG_XIP_PHYS_ADDR=0x08008000
CONFIG_BINFMT_FLAT=y
CONFIG_BINFMT_SHARED_FLAT=y
# CONFIG_COREDUMP is not set
+CONFIG_NET=y
+CONFIG_INET=y
+# CONFIG_INET_XFRM_MODE_TRANSPORT is not set
+# CONFIG_INET_XFRM_MODE_TUNNEL is not set
+# CONFIG_INET_XFRM_MODE_BEET is not set
+# CONFIG_IPV6 is not set
CONFIG_DEVTMPFS=y
CONFIG_DEVTMPFS_MOUNT=y
# CONFIG_FW_LOADER is not set
# CONFIG_BLK_DEV is not set
CONFIG_EEPROM_93CX6=y
+CONFIG_NETDEVICES=y
+CONFIG_STMMAC_ETH=y
+# CONFIG_WLAN is not set
# CONFIG_INPUT is not set
# CONFIG_SERIO is not set
# CONFIG_VT is not set
--
1.9.1
^ permalink raw reply related
* [PATCH v5 1/4] net: ethernet: dwmac: add Ethernet glue logic for stm32 chip
From: Alexandre TORGUE @ 2016-03-18 15:37 UTC (permalink / raw)
To: Maxime Coquelin, Giuseppe Cavallaro, netdev
Cc: linux-arm-kernel, linux-kernel, devicetree, robh, manabian
In-Reply-To: <1458315428-10081-1-git-send-email-alexandre.torgue@gmail.com>
stm324xx family chips support Synopsys MAC 3.510 IP.
This patch adds settings for logical glue logic:
-clocks
-mode selection MII or RMII.
Reviewed-by: Joachim Eastwood <manabian@gmail.com>
Acked-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Tested-by: Maxime Coquelin <maxime.coquelin@st.com>
Signed-off-by: Alexandre TORGUE <alexandre.torgue@gmail.com>
diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
index cec147d..235d679 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
+++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
@@ -104,6 +104,18 @@ config DWMAC_STI
device driver. This driver is used on for the STi series
SOCs GMAC ethernet controller.
+config DWMAC_STM32
+ tristate "STM32 DWMAC support"
+ default ARCH_STM32
+ depends on OF && HAS_IOMEM
+ select MFD_SYSCON
+ ---help---
+ Support for ethernet controller on STM32 SOCs.
+
+ This selects STM32 SoC glue layer support for the stmmac
+ device driver. This driver is used on for the STM32 series
+ SOCs GMAC ethernet controller.
+
config DWMAC_SUNXI
tristate "Allwinner GMAC support"
default ARCH_SUNXI
diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
index b390161..5f7ff0a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Makefile
+++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_DWMAC_MESON) += dwmac-meson.o
obj-$(CONFIG_DWMAC_ROCKCHIP) += dwmac-rk.o
obj-$(CONFIG_DWMAC_SOCFPGA) += dwmac-socfpga.o
obj-$(CONFIG_DWMAC_STI) += dwmac-sti.o
+obj-$(CONFIG_DWMAC_STM32) += dwmac-stm32.o
obj-$(CONFIG_DWMAC_SUNXI) += dwmac-sunxi.o
obj-$(CONFIG_DWMAC_GENERIC) += dwmac-generic.o
stmmac-platform-objs:= stmmac_platform.o
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
new file mode 100644
index 0000000..88c8573
--- /dev/null
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
@@ -0,0 +1,193 @@
+/*
+ * dwmac-stm32.c - DWMAC Specific Glue layer for STM32 MCU
+ *
+ * Copyright (C) Alexandre Torgue 2015
+ * Author: Alexandre Torgue <alexandre.torgue@gmail.com>
+ * License terms: GNU General Public License (GPL), version 2
+ *
+ */
+
+#include <linux/clk.h>
+#include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_net.h>
+#include <linux/phy.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <linux/stmmac.h>
+
+#include "stmmac_platform.h"
+
+#define MII_PHY_SEL_MASK BIT(23)
+
+struct stm32_dwmac {
+ struct clk *clk_tx;
+ struct clk *clk_rx;
+ u32 mode_reg; /* MAC glue-logic mode register */
+ struct regmap *regmap;
+ u32 speed;
+};
+
+static int stm32_dwmac_init(struct plat_stmmacenet_data *plat_dat)
+{
+ struct stm32_dwmac *dwmac = plat_dat->bsp_priv;
+ u32 reg = dwmac->mode_reg;
+ u32 val;
+ int ret;
+
+ val = (plat_dat->interface == PHY_INTERFACE_MODE_MII) ? 0 : 1;
+ ret = regmap_update_bits(dwmac->regmap, reg, MII_PHY_SEL_MASK, val);
+ if (ret)
+ return ret;
+
+ ret = clk_prepare_enable(dwmac->clk_tx);
+ if (ret)
+ return ret;
+
+ ret = clk_prepare_enable(dwmac->clk_rx);
+ if (ret)
+ clk_disable_unprepare(dwmac->clk_tx);
+
+ return ret;
+}
+
+static void stm32_dwmac_clk_disable(struct stm32_dwmac *dwmac)
+{
+ clk_disable_unprepare(dwmac->clk_tx);
+ clk_disable_unprepare(dwmac->clk_rx);
+}
+
+static int stm32_dwmac_parse_data(struct stm32_dwmac *dwmac,
+ struct device *dev)
+{
+ struct device_node *np = dev->of_node;
+ int err;
+
+ /* Get TX/RX clocks */
+ dwmac->clk_tx = devm_clk_get(dev, "tx-clk");
+ if (IS_ERR(dwmac->clk_tx)) {
+ dev_err(dev, "No tx clock provided...\n");
+ return PTR_ERR(dwmac->clk_tx);
+ }
+ dwmac->clk_rx = devm_clk_get(dev, "rx-clk");
+ if (IS_ERR(dwmac->clk_rx)) {
+ dev_err(dev, "No rx clock provided...\n");
+ return PTR_ERR(dwmac->clk_rx);
+ }
+
+ /* Get mode register */
+ dwmac->regmap = syscon_regmap_lookup_by_phandle(np, "st,syscon");
+ if (IS_ERR(dwmac->regmap))
+ return PTR_ERR(dwmac->regmap);
+
+ err = of_property_read_u32_index(np, "st,syscon", 1, &dwmac->mode_reg);
+ if (err)
+ dev_err(dev, "Can't get sysconfig mode offset (%d)\n", err);
+
+ return err;
+}
+
+static int stm32_dwmac_probe(struct platform_device *pdev)
+{
+ struct plat_stmmacenet_data *plat_dat;
+ struct stmmac_resources stmmac_res;
+ struct stm32_dwmac *dwmac;
+ int ret;
+
+ ret = stmmac_get_platform_resources(pdev, &stmmac_res);
+ if (ret)
+ return ret;
+
+ plat_dat = stmmac_probe_config_dt(pdev, &stmmac_res.mac);
+ if (IS_ERR(plat_dat))
+ return PTR_ERR(plat_dat);
+
+ dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL);
+ if (!dwmac)
+ return -ENOMEM;
+
+ ret = stm32_dwmac_parse_data(dwmac, &pdev->dev);
+ if (ret) {
+ dev_err(&pdev->dev, "Unable to parse OF data\n");
+ return ret;
+ }
+
+ plat_dat->bsp_priv = dwmac;
+
+ ret = stm32_dwmac_init(plat_dat);
+ if (ret)
+ return ret;
+
+ ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
+ if (ret)
+ stm32_dwmac_clk_disable(dwmac);
+
+ return ret;
+}
+
+static int stm32_dwmac_remove(struct platform_device *pdev)
+{
+ struct net_device *ndev = platform_get_drvdata(pdev);
+ struct stmmac_priv *priv = netdev_priv(ndev);
+ int ret = stmmac_dvr_remove(ndev);
+
+ stm32_dwmac_clk_disable(priv->plat->bsp_priv);
+
+ return ret;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int stm32_dwmac_suspend(struct device *dev)
+{
+ struct net_device *ndev = dev_get_drvdata(dev);
+ struct stmmac_priv *priv = netdev_priv(ndev);
+ int ret;
+
+ ret = stmmac_suspend(ndev);
+ stm32_dwmac_clk_disable(priv->plat->bsp_priv);
+
+ return ret;
+}
+
+static int stm32_dwmac_resume(struct device *dev)
+{
+ struct net_device *ndev = dev_get_drvdata(dev);
+ struct stmmac_priv *priv = netdev_priv(ndev);
+ int ret;
+
+ ret = stm32_dwmac_init(priv->plat);
+ if (ret)
+ return ret;
+
+ ret = stmmac_resume(ndev);
+
+ return ret;
+}
+#endif /* CONFIG_PM_SLEEP */
+
+SIMPLE_DEV_PM_OPS(stm32_dwmac_pm_ops, stm32_dwmac_suspend, stm32_dwmac_resume);
+
+static const struct of_device_id stm32_dwmac_match[] = {
+ { .compatible = "st,stm32-dwmac"},
+ { }
+};
+MODULE_DEVICE_TABLE(of, stm32_dwmac_match);
+
+static struct platform_driver stm32_dwmac_driver = {
+ .probe = stm32_dwmac_probe,
+ .remove = stm32_dwmac_remove,
+ .driver = {
+ .name = "stm32-dwmac",
+ .pm = &stm32_dwmac_pm_ops,
+ .of_match_table = stm32_dwmac_match,
+ },
+};
+module_platform_driver(stm32_dwmac_driver);
+
+MODULE_AUTHOR("Alexandre Torgue <alexandre.torgue@gmail.com>");
+MODULE_DESCRIPTION("STMicroelectronics MCU DWMAC Specific Glue layer");
+MODULE_LICENSE("GPL v2");
--
1.9.1
^ permalink raw reply related
* [PATCH v5 3/4] net: ethernet: stmmac: add support of Synopsys 3.50a MAC IP
From: Alexandre TORGUE @ 2016-03-18 15:37 UTC (permalink / raw)
To: Maxime Coquelin, Giuseppe Cavallaro, netdev
Cc: linux-arm-kernel, linux-kernel, devicetree, robh, manabian
In-Reply-To: <1458315428-10081-1-git-send-email-alexandre.torgue@gmail.com>
Adds support of Synopsys 3.50a MAC IP in stmmac driver.
Acked-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Tested-by: Maxime Coquelin <maxime.coquelin@st.com>
Signed-off-by: Alexandre TORGUE <alexandre.torgue@gmail.com>
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 6a52fa1..6cca626 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -178,6 +178,7 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
* once needed on other platforms.
*/
if (of_device_is_compatible(np, "st,spear600-gmac") ||
+ of_device_is_compatible(np, "snps,dwmac-3.50a") ||
of_device_is_compatible(np, "snps,dwmac-3.70a") ||
of_device_is_compatible(np, "snps,dwmac")) {
/* Note that the max-frame-size parameter as defined in the
--
1.9.1
^ permalink raw reply related
* [net-next v2] virtio_net: replace netdev_alloc_skb_ip_align() with napi_alloc_skb()
From: Paolo Abeni @ 2016-03-18 15:42 UTC (permalink / raw)
To: netdev
Cc: Michael S. Tsirkin, virtualization, Hannes Frederic Sowa,
Venkatesh Srinivas
This gives small but noticeable rx performance improvement (2-3%)
and will allow exploiting future napi improvement.
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
--
v2: replace also netdev_alloc_skb_ip_align() invocation in
add_recvbuf_small(), suggested by Venkatesh Srinivas
---
drivers/net/virtio_net.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index fb0eae4..100e039 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -260,7 +260,7 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
p = page_address(page) + offset;
/* copy small packet so we can reuse these pages for small data */
- skb = netdev_alloc_skb_ip_align(vi->dev, GOOD_COPY_LEN);
+ skb = napi_alloc_skb(&rq->napi, GOOD_COPY_LEN);
if (unlikely(!skb))
return NULL;
@@ -541,7 +541,7 @@ static int add_recvbuf_small(struct virtnet_info *vi, struct receive_queue *rq,
struct virtio_net_hdr_mrg_rxbuf *hdr;
int err;
- skb = __netdev_alloc_skb_ip_align(vi->dev, GOOD_PACKET_LEN, gfp);
+ skb = __napi_alloc_skb(&rq->napi, GOOD_PACKET_LEN, gfp);
if (unlikely(!skb))
return -ENOMEM;
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB
From: Sebastian Frias @ 2016-03-18 15:56 UTC (permalink / raw)
To: Uwe Kleine-König, Daniel Mack
Cc: David S. Miller, netdev, lkml, mason, Florian Fainelli,
Mans Rullgard, Fabio Estevam, Martin Blumenstingl, Linus Walleij
In-Reply-To: <20160318125455.GN4292@pengutronix.de>
Hi Uwe, Daniel,
On 03/18/2016 01:54 PM, Uwe Kleine-König wrote:
> [expand cc a bit more]
>
> Hello,
>
> On Wed, Mar 16, 2016 at 06:25:59PM +0100, Sebastian Frias wrote:
>> Commit 687908c2b649 ("net: phy: at803x: simplify using
>> devm_gpiod_get_optional and its 4th argument") introduced a dependency
>> on GPIOLIB that was not there before.
>>
>> This commit removes such dependency by checking the return code and
>> comparing it against ENOSYS which is returned when GPIOLIB is not
>> selected.
>>
>> Fixes: 687908c2b649 ("net: phy: at803x: simplify using devm_gpiod_get_optional and its 4th argument")
>>
>> Signed-off-by: Sebastian Frias <sf84@laposte.net>
>> ---
>> drivers/net/phy/at803x.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
>> index 2174ec9..88b7ff3 100644
>> --- a/drivers/net/phy/at803x.c
>> +++ b/drivers/net/phy/at803x.c
>> @@ -252,7 +252,9 @@ static int at803x_probe(struct phy_device *phydev)
>> return -ENOMEM;
>>
>> gpiod_reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
>> - if (IS_ERR(gpiod_reset))
>> + if (PTR_ERR(gpiod_reset) == -ENOSYS)
>> + gpiod_reset = NULL;
>> + else if (IS_ERR(gpiod_reset))
>
> this isn't better either (IMHO it's worse, but maybe this is debatable
> and also depends on your POV).
Well, from the code, the reset hack is only required when the PHY is
ATH8030_PHY_ID, right?
However, such change was introduced by Daniel Mack on commit 13a56b449325.
Hopefully he can chime in and give his opinion on this.
Daniel, while on the subject, I have a question for you:
Change 2b8f2a28eac1 introduced "link_status_notify" callback which is
called systematically on the PHY state machine.
Any reason to make the call systematic as opposed to let say:
if (phydev->state != old_state) {
if (phydev->drv->link_change_notify)
phydev->drv->link_change_notify(phydev);
}
(it does means that the callback would be called after the state machine
processing though)
>
> With 687908c2b649 I made kernels without GPIOLIB fail to bind this
> device. I admit this is not maximally nice.
I see, that was not clear from the commit message, sorry.
>
> Your change makes the driver bind in this case again and then the reset
> gpio isn't handled at all which might result in a later and harder to
> debug error.
>
> The better approach to fix your problem is: make the driver depend (or
> force) on GPIOLIB.
It was one of the solutions I had in mind, but:
- since the dependency on GPIOLIB was not included on the patch
- and given that the previous code had provision to work without GPIO
I thought it was an overlook.
>Or alternatively, drop reset-handling from the
> driver.
>
> From a driver perspecitive, it would be nice if devm_gpiod_get_optional
> returned NULL iff the respective gpio isn't specified even with
> GPIOLIB=n, but this isn't sensible either because it would result in
> quite some gpiolib code to not being conditionally compiled on
> CONFIG_GPIOLIB any more.
Let's say that was the case, what would the PHY code do?
I mean, it did not get a GPIO, whether it was because GPIOLIB=n or
because there's no 'reset' GPIO attached
Shall it fail? Shall it continue in a sort of degraded mode? Shall it warn?
Because that's the real question here.
What would you think of making at803x_link_change_notify() print a
message every time it should do a reset but does not has a way to do it?
I can make such change to my patch if everybody agrees on it.
By the way, in that case, can somebody suggest a way to print such warning?
Would printk() be ok or should I use dev_dbg() ?
Best regards,
Sebastian
>
> Best regards
> Uwe
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox