Netdev List
 help / color / mirror / Atom feed
* [PATCH iproute2-next 1/1] tc: jsonify tunnel_key action
From: Roman Mashak @ 2018-04-04 17:21 UTC (permalink / raw)
  To: dsahern; +Cc: stephen, netdev, kernel, jhs, xiyou.wangcong, jiri, Roman Mashak

Signed-off-by: Roman Mashak <mrv@mojatatu.com>
---
 tc/m_tunnel_key.c | 36 +++++++++++++++++++++++++-----------
 1 file changed, 25 insertions(+), 11 deletions(-)

diff --git a/tc/m_tunnel_key.c b/tc/m_tunnel_key.c
index bac3c07fa90b..0fa461549ad9 100644
--- a/tc/m_tunnel_key.c
+++ b/tc/m_tunnel_key.c
@@ -221,7 +221,13 @@ static void tunnel_key_print_ip_addr(FILE *f, const char *name,
 	else
 		return;
 
-	fprintf(f, "\n\t%s %s", name, rt_addr_n2a_rta(family, attr));
+	print_string(PRINT_FP, NULL, "%s", _SL_);
+	if (matches(name, "src_ip") == 0)
+		print_string(PRINT_ANY, "src_ip", "\tsrc_ip %s",
+			     rt_addr_n2a_rta(family, attr));
+	else if (matches(name, "dst_ip") == 0)
+		print_string(PRINT_ANY, "dst_ip", "\tdst_ip %s",
+			     rt_addr_n2a_rta(family, attr));
 }
 
 static void tunnel_key_print_key_id(FILE *f, const char *name,
@@ -229,7 +235,8 @@ static void tunnel_key_print_key_id(FILE *f, const char *name,
 {
 	if (!attr)
 		return;
-	fprintf(f, "\n\t%s %d", name, rta_getattr_be32(attr));
+	print_string(PRINT_FP, NULL, "%s", _SL_);
+	print_uint(PRINT_ANY, "key_id", "\tkey_id %u", rta_getattr_be32(attr));
 }
 
 static void tunnel_key_print_dst_port(FILE *f, char *name,
@@ -237,7 +244,9 @@ static void tunnel_key_print_dst_port(FILE *f, char *name,
 {
 	if (!attr)
 		return;
-	fprintf(f, "\n\t%s %d", name, rta_getattr_be16(attr));
+	print_string(PRINT_FP, NULL, "%s", _SL_);
+	print_uint(PRINT_ANY, "dst_port", "\tdst_port %u",
+		   rta_getattr_be16(attr));
 }
 
 static void tunnel_key_print_flag(FILE *f, const char *name_on,
@@ -246,7 +255,9 @@ static void tunnel_key_print_flag(FILE *f, const char *name_on,
 {
 	if (!attr)
 		return;
-	fprintf(f, "\n\t%s", rta_getattr_u8(attr) ? name_on : name_off);
+	print_string(PRINT_FP, NULL, "%s", _SL_);
+	print_string(PRINT_ANY, "flag", "\t%s",
+		     rta_getattr_u8(attr) ? name_on : name_off);
 }
 
 static int print_tunnel_key(struct action_util *au, FILE *f, struct rtattr *arg)
@@ -260,19 +271,20 @@ static int print_tunnel_key(struct action_util *au, FILE *f, struct rtattr *arg)
 	parse_rtattr_nested(tb, TCA_TUNNEL_KEY_MAX, arg);
 
 	if (!tb[TCA_TUNNEL_KEY_PARMS]) {
-		fprintf(f, "[NULL tunnel_key parameters]");
+		print_string(PRINT_FP, NULL, "%s",
+			     "[NULL tunnel_key parameters]");
 		return -1;
 	}
 	parm = RTA_DATA(tb[TCA_TUNNEL_KEY_PARMS]);
 
-	fprintf(f, "tunnel_key");
+	print_string(PRINT_ANY, "kind", "%s ", "tunnel_key");
 
 	switch (parm->t_action) {
 	case TCA_TUNNEL_KEY_ACT_RELEASE:
-		fprintf(f, " unset");
+		print_string(PRINT_ANY, "mode", " %s", "unset");
 		break;
 	case TCA_TUNNEL_KEY_ACT_SET:
-		fprintf(f, " set");
+		print_string(PRINT_ANY, "mode", " %s", "set");
 		tunnel_key_print_ip_addr(f, "src_ip",
 					 tb[TCA_TUNNEL_KEY_ENC_IPV4_SRC]);
 		tunnel_key_print_ip_addr(f, "dst_ip",
@@ -291,8 +303,10 @@ static int print_tunnel_key(struct action_util *au, FILE *f, struct rtattr *arg)
 	}
 	print_action_control(f, " ", parm->action, "");
 
-	fprintf(f, "\n\tindex %d ref %d bind %d", parm->index, parm->refcnt,
-		parm->bindcnt);
+	print_string(PRINT_FP, NULL, "%s", _SL_);
+	print_uint(PRINT_ANY, "index", "\t index %u", parm->index);
+	print_int(PRINT_ANY, "ref", " ref %d", parm->refcnt);
+	print_int(PRINT_ANY, "bind", " bind %d", parm->bindcnt);
 
 	if (show_stats) {
 		if (tb[TCA_TUNNEL_KEY_TM]) {
@@ -302,7 +316,7 @@ static int print_tunnel_key(struct action_util *au, FILE *f, struct rtattr *arg)
 		}
 	}
 
-	fprintf(f, "\n ");
+	print_string(PRINT_FP, NULL, "%s", _SL_);
 
 	return 0;
 }
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH v3 net 1/5] tcp: feed correct number of pkts acked to cc modules also in recovery
From: Neal Cardwell @ 2018-04-04 17:17 UTC (permalink / raw)
  To: Ilpo Järvinen; +Cc: Yuchung Cheng, Netdev, Eric Dumazet, sergei.shtylyov
In-Reply-To: <alpine.DEB.2.20.1803282331260.29108@whs-18.cs.helsinki.fi>

On Wed, Apr 4, 2018 at 6:42 AM Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
wrote:

> On Wed, 28 Mar 2018, Yuchung Cheng wrote:
...
> > Your patch looks good. Some questions / comments:

The patch looks good to me as well (I read the Mar 28 version). Thanks for
this fix, Ilpo.

> Just to be sure that we understand each other the correct way around this
> time, I guess it resolved both of your "disagree" points above?

> > 1. Why only apply to CA_Loss and not also CA_Recovery? this may
> > mitigate GRO issue I noted in other thread.

> Hmm, that's indeed a good idea. I'll give it some more thought but my
> initial impression is that it should work.

Great. I would agree with Yuchung's suggestion to apply this fix to
CA_Recovery as well.

> > 2. minor style nit: can we adjust tp->delivered directly in
> > tcp_clean_rtx_queue(). I am personally against calling "non-sack" reno
> > (e.g. tcp_*reno*()) because its really confusing w/ Reno congestion
> > control when SACK is used. One day I'd like to rename all these *reno*
> > to _nonsack_.

> That's what I actually did first but put ended up putting it into own
> function because of line lengths (not a particularly good reason).

> ...So yes, I can put it into the tcp_clean_rtx_queue in the next version.

> I'll also try to keep that "reno" thing in my mind.

Either approach here sounds fine to me. Though personally I find it more
readable to keep all the non-SACK helpers together and consistently named,
including this one (even if the name is confusing, at least it is
consistent... :-).

neal

^ permalink raw reply

* Re: [PATCH iproute2-next] tc: Correct json output for actions
From: David Ahern @ 2018-04-04 17:13 UTC (permalink / raw)
  To: Yuval Mintz; +Cc: mlxsw, netdev, Stephen Hemminger, Roman Mashak
In-Reply-To: <1522844653-37136-1-git-send-email-yuvalm@mellanox.com>

On 4/4/18 6:24 AM, Yuval Mintz wrote:
> Commit 9fd3f0b255d9 ("tc: enable json output for actions") added JSON
> support for tc-actions at the expense of breaking other use cases that
> reach tc_print_action(), as the latter don't expect the 'actions' array
> to be a new object.
> 
> Consider the following taken duringrun of tc_chain.sh selftest,
> and see the latter command output is broken:
> 
> $ ./tc/tc -j -p actions list action gact | grep -C 3 actions
> [ {
>         "total acts": 1
>     },{
>         "actions": [ {
>                 "order": 0,
> 
> $ ./tc/tc -p -j -s filter show dev enp3s0np2 ingress | grep -C 3 actions
>             },
>             "skip_hw": true,
>             "not_in_hw": true,{
>                 "actions": [ {
>                         "order": 1,
>                         "kind": "gact",
>                         "control_action": {
> 
> Relocate the open/close of the JSON object to declare the object only
> for the case that needs it.
> 
> Signed-off-by: Yuval Mintz <yuvalm@mellanox.com>
> ---
>  tc/m_action.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)


This is a bug fix so it should go through master.

Added Roman so he is aware of the mistake.

^ permalink raw reply

* Re: [PATCH v3 net 2/5] tcp: prevent bogus FRTO undos with non-SACK flows
From: Yuchung Cheng @ 2018-04-04 17:12 UTC (permalink / raw)
  To: Neal Cardwell; +Cc: Ilpo Järvinen, Netdev, Eric Dumazet, Sergei Shtylyov
In-Reply-To: <CADVnQykPMCDzkUEtgKR3jGp7pnSyoO2p+c3oogJ8yih-EchX7w@mail.gmail.com>

On Wed, Apr 4, 2018 at 9:33 AM, Neal Cardwell <ncardwell@google.com> wrote:
>
> On Wed, Apr 4, 2018 at 6:35 AM Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
> wrote:
>
> > On Wed, 28 Mar 2018, Yuchung Cheng wrote:
>
> > > On Tue, Mar 13, 2018 at 3:25 AM, Ilpo Järvinen
> > > <ilpo.jarvinen@helsinki.fi> wrote:
> > > >
> > > > If SACK is not enabled and the first cumulative ACK after the RTO
> > > > retransmission covers more than the retransmitted skb, a spurious
> > > > FRTO undo will trigger (assuming FRTO is enabled for that RTO).
> > > > The reason is that any non-retransmitted segment acknowledged will
> > > > set FLAG_ORIG_SACK_ACKED in tcp_clean_rtx_queue even if there is
> > > > no indication that it would have been delivered for real (the
> > > > scoreboard is not kept with TCPCB_SACKED_ACKED bits in the non-SACK
> > > > case so the check for that bit won't help like it does with SACK).
> > > > Having FLAG_ORIG_SACK_ACKED set results in the spurious FRTO undo
> > > > in tcp_process_loss.
> > > >
> > > > We need to use more strict condition for non-SACK case and check
> > > > that none of the cumulatively ACKed segments were retransmitted
> > > > to prove that progress is due to original transmissions. Only then
> > > > keep FLAG_ORIG_SACK_ACKED set, allowing FRTO undo to proceed in
> > > > non-SACK case.
> > > >
> > > > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
> > > > ---
> > > >  net/ipv4/tcp_input.c | 9 +++++++++
> > > >  1 file changed, 9 insertions(+)
> > > >
> > > > diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> > > > index 4a26c09..c60745c 100644
> > > > --- a/net/ipv4/tcp_input.c
> > > > +++ b/net/ipv4/tcp_input.c
> > > > @@ -3166,6 +3166,15 @@ static int tcp_clean_rtx_queue(struct sock
> *sk, u32 prior_fack,
> > > >                                 pkts_acked = rexmit_acked +
> newdata_acked;
> > > >
> > > >                         tcp_remove_reno_sacks(sk, pkts_acked);
> > > > +
> > > > +                       /* If any of the cumulatively ACKed segments
> was
> > > > +                        * retransmitted, non-SACK case cannot
> confirm that
> > > > +                        * progress was due to original transmission
> due to
> > > > +                        * lack of TCPCB_SACKED_ACKED bits even if
> some of
> > > > +                        * the packets may have been never
> retransmitted.
> > > > +                        */
> > > > +                       if (flag & FLAG_RETRANS_DATA_ACKED)
> > > > +                               flag &= ~FLAG_ORIG_SACK_ACKED;
>
> FWIW I'd vote for this version.
>
> > Of course I could put the back there but I really like the new place more
> > (which was a result of your suggestion to place the code elsewhere).
> > IMHO, it makes more sense to have it in tcp_clean_rtx_queue() because we
> > weren't successful in proving (there in tcp_clean_rtx_queue) that progress
> > was due original transmission and thus I would not want falsely indicate
> > it with that flag. And there's the non-SACK related block anyway already
> > there so it keeps the non-SACK "pollution" off from the SACK code paths.
>
> I think that's a compelling argument. In particular, in terms of long-term
> maintenance it seems risky to allow an unsound/buggy FLAG_ORIG_SACK_ACKED
> bit be returned by tcp_clean_rtx_queue(). If we return an
> incorrect/imcomplete FLAG_ORIG_SACK_ACKED bit then I worry that one day we
> will forget that for non-SACK flows that bit is incorrect/imcomplete, and
> we will add code using that bit but forgetting to check (tcp_is_sack(tp) ||
> !FLAG_RETRANS_DATA_ACKED).
Agreed. That's a good point. And I would much preferred to rename that
to FLAG_ORIG_PROGRESS (w/ updated comment).

so I think we're in agreement to use existing patch w/ the new name
FLAG_ORIG_PROGRESS

>
> > (In addition, I'd actually also like to rename FLAG_ORIG_SACK_ACKED to
> > FLAG_ORIG_PROGRESS, the latter is more descriptive about the condition
> > we're after regardless of SACK and less ambiguous in non-SACK case).
>
> I'm neutral on this. Not sure if the extra clarity is worth the code churn.
>
> cheers,
> neal

^ permalink raw reply

* Re: [PATCH net] arp: fix arp_filter on l3slave devices
From: David Ahern @ 2018-04-04 17:11 UTC (permalink / raw)
  To: Miguel Fadon Perlines, netdev@vger.kernel.org
In-Reply-To: <HE1PR07MB087563C78F897EABB6B8F326B6A40@HE1PR07MB0875.eurprd07.prod.outlook.com>

On 4/4/18 4:13 AM, Miguel Fadon Perlines wrote:
> arp_filter performs an ip_route_output search for arp source address and
> 
> checks if output device is the same where the arp request was received,
> 
> if it is not, the arp request is not answered.
> 
>  
> 
> This route lookup is always done on main route table so l3slave devices
> 
> never find the proper route and arp is not answered.
> 
>  
> 
> Passing l3mdev_master_ifindex_rcu(dev) return value as oif fixes the
> 
> lookup for l3slave devices while maintaining same behavior for non
> 
> l3slave devices as this function returns 0 in that case.
> 
>  
> 
> Signed-off-by: Miguel Fadon Perlines <mfadon@teldat.com>
> 
> ---

Miguel: The change looks fine to me. Your mail showed up as html; it
needs to be sent as plain text only.

^ permalink raw reply

* Re: [Intel-wired-lan] [iwl next-queue PATCH 03/10] macvlan: Use software path for offloaded local, broadcast, and multicast traffic
From: Alexander Duyck @ 2018-04-04 17:02 UTC (permalink / raw)
  To: Shannon Nelson; +Cc: Alexander Duyck, intel-wired-lan, Jeff Kirsher, Netdev
In-Reply-To: <a99a73c2-1216-30ba-9d86-9bb4626a8a81@oracle.com>

On Wed, Apr 4, 2018 at 9:53 AM, Shannon Nelson
<shannon.nelson@oracle.com> wrote:
> On 4/3/2018 2:16 PM, Alexander Duyck wrote:
>>
>> This change makes it so that we use a software path for packets that are
>> going to be locally switched between two macvlan interfaces on the same
>> device. In addition we resort to software replication of broadcast and
>> multicast packets instead of offloading that to hardware.
>>
>> The general idea is that using the device for east/west traffic local to
>> the system is extremely inefficient. We can only support up to whatever
>> the
>> PCIe limit is for any given device so this caps us at somewhere around 20G
>> for devices supported by ixgbe. This is compounded even further when you
>> take broadcast and multicast into account as a single 10G port can come to
>> a crawl as a packet is replicated up to 60+ times in some cases. In order
>> to get away from that I am implementing changes so that we handle
>> broadcast/multicast replication and east/west local traffic all in
>> software.
>>
>> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
>> ---
>
>
> [...]
>
>>   @@ -4225,6 +4226,13 @@ static void ixgbe_configure_virtualization(struct
>> ixgbe_adapter *adapter)
>>         vmdctl |= IXGBE_VT_CTL_REPLEN;
>>         IXGBE_WRITE_REG(hw, IXGBE_VT_CTL, vmdctl);
>>   +     /* accept untagged packets until a vlan tag is
>> +        * specifically set for the VMDQ queue/pool
>> +        */
>> +       vmolr = IXGBE_VMOLR_AUPE;
>> +       while (pool--)
>> +               IXGBE_WRITE_REG(hw, IXGBE_VMOLR(VMDQ_P(pool)), vmolr);
>> +
>
>
> It took me a bit to figure out what untagged packets have to do with macvlan
> config.  You might add to the comment that "no multicast or broadcast is
> configured for these queues".

Yeah. I can probably address that in the next round of patches.
Specifically here all we are accepting are untagged unicast packets.
We cannot perform the offload through a VLAN since it would require
passing the VLAN along with the L2 MAC address. It is something I plan
to address in the near future so that macvlan interfaces stacked on
top of a VLAN can still be offloaded.

> The driver part of this might be separated into a follow-on patch to make it
> clearer that this is done as a consequence of the macvlan.c changes.  Or
> just roll them into your 4/10 patch.
>
> sln

Actually part of the reason for keeping this all as one thing is
because if I split the macvlan bits from the driver bits then things
are broken until both patches are applied.

I had actually split patch 4/10 out from this patch since it is one of
the few things that could safely be pulled out in order to try and
reduce the overhead for this patch.

^ permalink raw reply

* Re: [PATCH] netdevsim: remove incorrect __net_initdata annotations
From: David Miller @ 2018-04-04 16:54 UTC (permalink / raw)
  To: arnd; +Cc: jakub.kicinski, dsa, netdev, linux-kernel
In-Reply-To: <CAK8P3a0um2irY2P_3xffqRcmu5mJ+iA9iO4eW5381zBBmPRQfg@mail.gmail.com>

From: Arnd Bergmann <arnd@arndb.de>
Date: Wed, 4 Apr 2018 18:03:41 +0200

> On Wed, Apr 4, 2018 at 5:52 PM, David Miller <davem@davemloft.net> wrote:
>> From: Arnd Bergmann <arnd@arndb.de>
>> Date: Wed,  4 Apr 2018 14:12:39 +0200
>>
>>> The __net_initdata section cannot currently be used for structures that
>>> get cleaned up in an exitcall using unregister_pernet_operations:
>>>
>>> WARNING: vmlinux.o(.text+0x868c34): Section mismatch in reference from the function nsim_devlink_exit() to the (unknown reference) .init.data:(unknown)
>>> The function nsim_devlink_exit() references
>>> the (unknown reference) __initdata (unknown).
>>> This is often because nsim_devlink_exit lacks a __initdata
>>> annotation or the annotation of (unknown) is wrong.
>>> WARNING: vmlinux.o(.text+0x868c64): Section mismatch in reference from the function nsim_devlink_init() to the (unknown reference) .init.data:(unknown)
>>> WARNING: vmlinux.o(.text+0x8692bc): Section mismatch in reference from the function nsim_fib_exit() to the (unknown reference) .init.data:(unknown)
>>> WARNING: vmlinux.o(.text+0x869300): Section mismatch in reference from the function nsim_fib_init() to the (unknown reference) .init.data:(unknown)
>>>
>>> As that warning tells us, discarding the structure after a module is
>>> loaded would lead to a undefined behavior when that module is removed.
>>>
>>> It might be possible to change that annotation so it has no effect for
>>> loadable modules, but I have not figured out exactly how to do that, and
>>> we want this to be fixed in -rc1.
>>>
>>> This just removes the annotations, just like we do for all other such
>>> modules.
>>>
>>> Fixes: 37923ed6b8ce ("netdevsim: Add simple FIB resource controller via devlink")
>>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>>
>> Hmmm...
>>
>> __net_initdata defines to nothing if network namespaces are enabled.
>>
>> That's the whole point.
>>
>> Therefore, if NETNS is disabled, "unregister_pernet_operations" cannot
>> happen and this is the only time when __net_initdata actually does
>> something.
> 
> The problem happens with CONFIG_NETNS=n: __net_initdata turns into
> __initdata, and nsim_fib_net_ops is referenced from a function that is
> not marked __init (i.e. nsim_fib_exit).
> 
> The logic to turn __net_initdata into __init only makes sense for
> subsystems that have no way to be unregistered, i.e. code which is
> always built-in, or non-unloadable modules.

Ok, I see it.

I'll apply this for now, thanks.

^ permalink raw reply

* Re: [Intel-wired-lan] [iwl next-queue PATCH 02/10] macvlan: Rename fwd_priv to accel_priv and add accessor function
From: Shannon Nelson @ 2018-04-04 16:53 UTC (permalink / raw)
  To: Alexander Duyck, intel-wired-lan, jeffrey.t.kirsher; +Cc: netdev
In-Reply-To: <20180403211603.7880.2593.stgit@ahduyck-green-test.jf.intel.com>

On 4/3/2018 2:16 PM, Alexander Duyck wrote:

[...]
>   
> +static inline void *macvlan_accel_priv(struct net_device *dev)
> +{
> +	struct macvlan_dev *macvlan = netdev_priv(dev);
> +
> +	return macvlan->accel_priv;

Perhaps a check for (macvlan == NULL) before using it?
sln

> +}
>   #endif /* _LINUX_IF_MACVLAN_H */
> 
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan@osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
> 

^ permalink raw reply

* Re: [Intel-wired-lan] [iwl next-queue PATCH 03/10] macvlan: Use software path for offloaded local, broadcast, and multicast traffic
From: Shannon Nelson @ 2018-04-04 16:53 UTC (permalink / raw)
  To: Alexander Duyck, intel-wired-lan, jeffrey.t.kirsher; +Cc: netdev
In-Reply-To: <20180403211609.7880.1015.stgit@ahduyck-green-test.jf.intel.com>

On 4/3/2018 2:16 PM, Alexander Duyck wrote:
> This change makes it so that we use a software path for packets that are
> going to be locally switched between two macvlan interfaces on the same
> device. In addition we resort to software replication of broadcast and
> multicast packets instead of offloading that to hardware.
> 
> The general idea is that using the device for east/west traffic local to
> the system is extremely inefficient. We can only support up to whatever the
> PCIe limit is for any given device so this caps us at somewhere around 20G
> for devices supported by ixgbe. This is compounded even further when you
> take broadcast and multicast into account as a single 10G port can come to
> a crawl as a packet is replicated up to 60+ times in some cases. In order
> to get away from that I am implementing changes so that we handle
> broadcast/multicast replication and east/west local traffic all in
> software.
> 
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> ---

[...]

>   
> @@ -4225,6 +4226,13 @@ static void ixgbe_configure_virtualization(struct ixgbe_adapter *adapter)
>   	vmdctl |= IXGBE_VT_CTL_REPLEN;
>   	IXGBE_WRITE_REG(hw, IXGBE_VT_CTL, vmdctl);
>   
> +	/* accept untagged packets until a vlan tag is
> +	 * specifically set for the VMDQ queue/pool
> +	 */
> +	vmolr = IXGBE_VMOLR_AUPE;
> +	while (pool--)
> +		IXGBE_WRITE_REG(hw, IXGBE_VMOLR(VMDQ_P(pool)), vmolr);
> +

It took me a bit to figure out what untagged packets have to do with 
macvlan config.  You might add to the comment that "no multicast or 
broadcast is configured for these queues".

The driver part of this might be separated into a follow-on patch to 
make it clearer that this is done as a consequence of the macvlan.c 
changes.  Or just roll them into your 4/10 patch.

sln

^ permalink raw reply

* Re: [PATCH v3 net 2/5] tcp: prevent bogus FRTO undos with non-SACK flows
From: Neal Cardwell @ 2018-04-04 16:33 UTC (permalink / raw)
  To: Ilpo Järvinen; +Cc: Yuchung Cheng, Netdev, Eric Dumazet, sergei.shtylyov
In-Reply-To: <alpine.DEB.2.20.1804041324031.29120@whs-18.cs.helsinki.fi>

On Wed, Apr 4, 2018 at 6:35 AM Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
wrote:

> On Wed, 28 Mar 2018, Yuchung Cheng wrote:

> > On Tue, Mar 13, 2018 at 3:25 AM, Ilpo Järvinen
> > <ilpo.jarvinen@helsinki.fi> wrote:
> > >
> > > If SACK is not enabled and the first cumulative ACK after the RTO
> > > retransmission covers more than the retransmitted skb, a spurious
> > > FRTO undo will trigger (assuming FRTO is enabled for that RTO).
> > > The reason is that any non-retransmitted segment acknowledged will
> > > set FLAG_ORIG_SACK_ACKED in tcp_clean_rtx_queue even if there is
> > > no indication that it would have been delivered for real (the
> > > scoreboard is not kept with TCPCB_SACKED_ACKED bits in the non-SACK
> > > case so the check for that bit won't help like it does with SACK).
> > > Having FLAG_ORIG_SACK_ACKED set results in the spurious FRTO undo
> > > in tcp_process_loss.
> > >
> > > We need to use more strict condition for non-SACK case and check
> > > that none of the cumulatively ACKed segments were retransmitted
> > > to prove that progress is due to original transmissions. Only then
> > > keep FLAG_ORIG_SACK_ACKED set, allowing FRTO undo to proceed in
> > > non-SACK case.
> > >
> > > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
> > > ---
> > >  net/ipv4/tcp_input.c | 9 +++++++++
> > >  1 file changed, 9 insertions(+)
> > >
> > > diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> > > index 4a26c09..c60745c 100644
> > > --- a/net/ipv4/tcp_input.c
> > > +++ b/net/ipv4/tcp_input.c
> > > @@ -3166,6 +3166,15 @@ static int tcp_clean_rtx_queue(struct sock
*sk, u32 prior_fack,
> > >                                 pkts_acked = rexmit_acked +
newdata_acked;
> > >
> > >                         tcp_remove_reno_sacks(sk, pkts_acked);
> > > +
> > > +                       /* If any of the cumulatively ACKed segments
was
> > > +                        * retransmitted, non-SACK case cannot
confirm that
> > > +                        * progress was due to original transmission
due to
> > > +                        * lack of TCPCB_SACKED_ACKED bits even if
some of
> > > +                        * the packets may have been never
retransmitted.
> > > +                        */
> > > +                       if (flag & FLAG_RETRANS_DATA_ACKED)
> > > +                               flag &= ~FLAG_ORIG_SACK_ACKED;

FWIW I'd vote for this version.

> Of course I could put the back there but I really like the new place more
> (which was a result of your suggestion to place the code elsewhere).
> IMHO, it makes more sense to have it in tcp_clean_rtx_queue() because we
> weren't successful in proving (there in tcp_clean_rtx_queue) that progress
> was due original transmission and thus I would not want falsely indicate
> it with that flag. And there's the non-SACK related block anyway already
> there so it keeps the non-SACK "pollution" off from the SACK code paths.

I think that's a compelling argument. In particular, in terms of long-term
maintenance it seems risky to allow an unsound/buggy FLAG_ORIG_SACK_ACKED
bit be returned by tcp_clean_rtx_queue(). If we return an
incorrect/imcomplete FLAG_ORIG_SACK_ACKED bit then I worry that one day we
will forget that for non-SACK flows that bit is incorrect/imcomplete, and
we will add code using that bit but forgetting to check (tcp_is_sack(tp) ||
!FLAG_RETRANS_DATA_ACKED).

> (In addition, I'd actually also like to rename FLAG_ORIG_SACK_ACKED to
> FLAG_ORIG_PROGRESS, the latter is more descriptive about the condition
> we're after regardless of SACK and less ambiguous in non-SACK case).

I'm neutral on this. Not sure if the extra clarity is worth the code churn.

cheers,
neal

^ permalink raw reply

* (unknown), 
From:  системы администратор @ 2018-04-04 13:43 UTC (permalink / raw)




пользователь веб-почты

Обратите внимание, что 95% ваших писем, полученных после обновления сервера веб-почты в последнее время в нашей базе данных, были отложены. Регулярно получать и отправлять свои сообщения. Техническая команда нашей веб-почты обновит вашу учетную запись в течение 3 рабочих дней. Если вы этого не сделаете, ваша учетная запись будет временно приостановлена нашими службами. Чтобы повторно подтвердить свой почтовый ящик, пришлите следующую
  информацию:

обычный:
Имя пользователя:
пароль:
Подтвердите Пароль:

Предупреждение!! Если это не позволит обновлять учетные записи в
течение двух дней после получения электронной почты, они будут
постоянно потерять учетные записи владельцев учетных записей
веб-почты.

Спасибо за ваше сотрудничество!

Copyright &copy; 2017-2018 Служба технической поддержки WebMail, Inc.

^ permalink raw reply

* Re: [PATCH v2 0/2] gfs2: Stop using rhashtable_walk_peek
From: Andreas Grünbacher @ 2018-04-04 16:10 UTC (permalink / raw)
  To: Herbert Xu
  Cc: netdev, NeilBrown, linux-kernel, cluster-devel, Thomas Graf,
	Tom Herbert
In-Reply-To: <20180404154859.GA12104@gondor.apana.org.au>

[-- Attachment #1: Type: text/plain, Size: 537 bytes --]

Herbert Xu <herbert@gondor.apana.org.au> schrieb am Mi. 4. Apr. 2018 um
17:51:

> On Wed, Apr 04, 2018 at 11:46:28AM -0400, Bob Peterson wrote:
> >
> > The patches look good. The big question is whether to add them to this
> > merge window while it's still open. Opinions?
>
> We're still hashing out the rhashtable interface so I don't think now is
> the time to rush things.


Fair enough. No matter how rhashtable_walk_peek changes, we‘ll still need
these two patches to fix the glock dump though.

Thanks,
Andreas

[-- Attachment #2: Type: text/html, Size: 907 bytes --]

^ permalink raw reply

* Re: [PATCH net] sfc: remove ctpio_dmabuf_start from stats
From: David Miller @ 2018-04-04 16:08 UTC (permalink / raw)
  To: bkenward; +Cc: netdev, linux-net-drivers
In-Reply-To: <ba10b4db-696a-ee0d-82e0-15770a69b01f@solarflare.com>

From: Bert Kenward <bkenward@solarflare.com>
Date: Wed, 4 Apr 2018 16:40:30 +0100

> The ctpio_dmabuf_start entry is not actually a stat and shouldn't
> be exposed to ethtool.
> 
> Fixes: 2c0b6ee837db ("sfc: expose CTPIO stats on NICs that support
> them")

Please don't break up a long Fixes tag line into multiple lines.  I
corrected it this time.

> Signed-off-by: Bert Kenward <bkenward@solarflare.com>

Applied, thanks.

^ permalink raw reply

* Commits going to plain 'net' tree now.
From: David Miller @ 2018-04-04 16:06 UTC (permalink / raw)
  To: netdev


Therefore, please submit bug fixes relative to 'net', thank you.

^ permalink raw reply

* Re: [PATCH net-next 2/2] inet: frags: fix ip6frag_low_thresh boundary
From: David Miller @ 2018-04-04 16:05 UTC (permalink / raw)
  To: edumazet; +Cc: netdev, eric.dumazet
In-Reply-To: <20180404153510.213392-1-edumazet@google.com>

From: Eric Dumazet <edumazet@google.com>
Date: Wed,  4 Apr 2018 08:35:10 -0700

> Giving an integer to proc_doulongvec_minmax() is dangerous on 64bit arches,
> since linker might place next to it a non zero value preventing a change
> to ip6frag_low_thresh.
> 
> ip6frag_low_thresh is not used anymore in the kernel, but we do not
> want to prematuraly break user scripts wanting to change it.
> 
> Since specifying a minimal value of 0 for proc_doulongvec_minmax()
> is moot, let's remove these zero values in all defrag units.
> 
> Fixes: 6e00f7dd5e4e ("ipv6: frags: fix /proc/sys/net/ipv6/ip6frag_low_thresh")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Maciej Żenczykowski <maze@google.com>

Applied, thanks Eric.

^ permalink raw reply

* Re: [PATCH] netdevsim: remove incorrect __net_initdata annotations
From: Arnd Bergmann @ 2018-04-04 16:03 UTC (permalink / raw)
  To: David Miller
  Cc: Jakub Kicinski, David Ahern, Networking,
	Linux Kernel Mailing List
In-Reply-To: <20180404.115215.419039708931754897.davem@davemloft.net>

On Wed, Apr 4, 2018 at 5:52 PM, David Miller <davem@davemloft.net> wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> Date: Wed,  4 Apr 2018 14:12:39 +0200
>
>> The __net_initdata section cannot currently be used for structures that
>> get cleaned up in an exitcall using unregister_pernet_operations:
>>
>> WARNING: vmlinux.o(.text+0x868c34): Section mismatch in reference from the function nsim_devlink_exit() to the (unknown reference) .init.data:(unknown)
>> The function nsim_devlink_exit() references
>> the (unknown reference) __initdata (unknown).
>> This is often because nsim_devlink_exit lacks a __initdata
>> annotation or the annotation of (unknown) is wrong.
>> WARNING: vmlinux.o(.text+0x868c64): Section mismatch in reference from the function nsim_devlink_init() to the (unknown reference) .init.data:(unknown)
>> WARNING: vmlinux.o(.text+0x8692bc): Section mismatch in reference from the function nsim_fib_exit() to the (unknown reference) .init.data:(unknown)
>> WARNING: vmlinux.o(.text+0x869300): Section mismatch in reference from the function nsim_fib_init() to the (unknown reference) .init.data:(unknown)
>>
>> As that warning tells us, discarding the structure after a module is
>> loaded would lead to a undefined behavior when that module is removed.
>>
>> It might be possible to change that annotation so it has no effect for
>> loadable modules, but I have not figured out exactly how to do that, and
>> we want this to be fixed in -rc1.
>>
>> This just removes the annotations, just like we do for all other such
>> modules.
>>
>> Fixes: 37923ed6b8ce ("netdevsim: Add simple FIB resource controller via devlink")
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> Hmmm...
>
> __net_initdata defines to nothing if network namespaces are enabled.
>
> That's the whole point.
>
> Therefore, if NETNS is disabled, "unregister_pernet_operations" cannot
> happen and this is the only time when __net_initdata actually does
> something.

The problem happens with CONFIG_NETNS=n: __net_initdata turns into
__initdata, and nsim_fib_net_ops is referenced from a function that is
not marked __init (i.e. nsim_fib_exit).

The logic to turn __net_initdata into __init only makes sense for
subsystems that have no way to be unregistered, i.e. code which is
always built-in, or non-unloadable modules.

       Arnd

^ permalink raw reply

* Re: [PATCH net-next] vxlan: add ttl inherit support
From: David Miller @ 2018-04-04 16:03 UTC (permalink / raw)
  To: liuhangbin; +Cc: netdev, jbenc, lucien.xin
In-Reply-To: <1522848381-9934-1-git-send-email-liuhangbin@gmail.com>

From: Hangbin Liu <liuhangbin@gmail.com>
Date: Wed,  4 Apr 2018 21:26:21 +0800

> Like tos inherit, ttl inherit should also means inherit the inner protocol's
> ttl values, which actually not implemented in vxlan yet.
> 
> But we could not treat ttl == 0 as "use the inner TTL", because that would be
> used also when the "ttl" option is not specified and that would be a behavior
> change, and breaking real use cases.
> 
> So add a different attribute IFLA_VXLAN_TTL_INHERIT when "ttl inherit" is
> specified with ip cmd.
> 
> Reported-by: Jianlin Shi <jishi@redhat.com>
> Suggested-by: Jiri Benc <jbenc@redhat.com>
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>

The net-next tree is closed, please resubmit this when the net-next tree opens
back up.

Thank you.

^ permalink raw reply

* Re: WARNING: refcount bug in should_fail
From: Eric W. Biederman @ 2018-04-04 15:54 UTC (permalink / raw)
  To: Al Viro
  Cc: Tetsuo Handa, syzbot+, syzkaller-bugs, dvyukov, linux-fsdevel,
	linux-mm, netdev
In-Reply-To: <20180403052009.GH30522@ZenIV.linux.org.uk>

Al Viro <viro@ZenIV.linux.org.uk> writes:

> On Mon, Apr 02, 2018 at 10:59:34PM +0100, Al Viro wrote:
>
>> FWIW, I'm going through the ->kill_sb() instances, fixing that sort
>> of bugs (most of them preexisting, but I should've checked instead
>> of assuming that everything's fine).  Will push out later tonight.
>
> OK, see vfs.git#for-linus.  Caught: 4 old bugs (allocation failure
> in fill_super oopses ->kill_sb() in hypfs, jffs2 and orangefs resp.
> and double-dput in late failure exit in rpc_fill_super())
> and 5 regressions from register_shrinker() failure recovery.

One issue with your vfs.git#for-linus branch.

It is missing Fixes tags and  Cc: stable on those patches.
As the bug came in v4.15 those tags would really help the stable
maintainers get the recent regression fixes applied.

Eric

^ permalink raw reply

* Re: [net-next 1/1] tipc: Fix namespace violation in tipc_sk_fill_sock_diag
From: David Miller @ 2018-04-04 15:55 UTC (permalink / raw)
  To: mohan.krishna.ghanta.krishnamurthy
  Cc: tipc-discussion, jon.maloy, maloy, ying.xue, netdev
In-Reply-To: <1522846187-23914-1-git-send-email-mohan.krishna.ghanta.krishnamurthy@ericsson.com>

From: GhantaKrishnamurthy MohanKrishna <mohan.krishna.ghanta.krishnamurthy@ericsson.com>
Date: Wed, 4 Apr 2018 14:49:47 +0200

> To fetch UID info for socket diagnostics, we determine the
> namespace of user context using tipc socket instance. This
> may cause namespace violation, as the kernel will remap based
> on UID.
> 
> We fix this by fetching namespace info using the calling userspace
> netlink socket.
> 
> Fixes: c30b70deb5f4 (tipc: implement socket diagnostics for AF_TIPC)
> Reported-by: syzbot+326e587eff1074657718@syzkaller.appspotmail.com
> Acked-by: Jon Maloy <jon.maloy@ericsson.com>
> Signed-off-by: GhantaKrishnamurthy MohanKrishna <mohan.krishna.ghanta.krishnamurthy@ericsson.com>

Applied, thank you.

^ permalink raw reply

* Re: [PATCH net-next] net: avoid unneeded atomic operation in ip*_append_data()
From: David Miller @ 2018-04-04 15:54 UTC (permalink / raw)
  To: pabeni; +Cc: netdev, edumazet
In-Reply-To: <7e55c93c2c7cddf4c077aa77aa1ab58396f502ff.1522844999.git.pabeni@redhat.com>

From: Paolo Abeni <pabeni@redhat.com>
Date: Wed,  4 Apr 2018 14:30:01 +0200

> After commit 694aba690de0 ("ipv4: factorize sk_wmem_alloc updates
> done by __ip_append_data()") and commit 1f4c6eb24029 ("ipv6:
> factorize sk_wmem_alloc updates done by __ip6_append_data()"),
> when transmitting sub MTU datagram, an addtional, unneeded atomic
> operation is performed in ip*_append_data() to update wmem_alloc:
> in the above condition the delta is 0.
> 
> The above cause small but measurable performance regression in UDP
> xmit tput test with packet size below MTU.
> 
> This change avoids such overhead updating wmem_alloc only if
> wmem_alloc_delta is non zero.
> 
> The error path is left intentionally unmodified: it's a slow path
> and simplicity is preferred to performances.
> 
> Fixes: 694aba690de0 ("ipv4: factorize sk_wmem_alloc updates done by __ip_append_data()")
> Fixes: 1f4c6eb24029 ("ipv6: factorize sk_wmem_alloc updates done by __ip6_append_data()")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
 ...
> -	refcount_add(wmem_alloc_delta, &sk->sk_wmem_alloc);
> +	if (wmem_alloc_delta)
> +		refcount_add(wmem_alloc_delta, &sk->sk_wmem_alloc);
 ...
> -	refcount_add(wmem_alloc_delta, &sk->sk_wmem_alloc);
> +	if (wmem_alloc_delta)
> +		refcount_add(wmem_alloc_delta, &sk->sk_wmem_alloc);

This is simple enough, so applied.

But I wonder if atomic_{add,sub} and refcount_{add,sub}() should just check
for zero inline, just like the {set,clear}_bit() implementations avoid the
atomic operation if the bit already has the desired value.

^ permalink raw reply

* Re: [PATCH] netdevsim: remove incorrect __net_initdata annotations
From: David Miller @ 2018-04-04 15:52 UTC (permalink / raw)
  To: arnd; +Cc: jakub.kicinski, dsa, netdev, linux-kernel
In-Reply-To: <20180404121347.3089169-1-arnd@arndb.de>

From: Arnd Bergmann <arnd@arndb.de>
Date: Wed,  4 Apr 2018 14:12:39 +0200

> The __net_initdata section cannot currently be used for structures that
> get cleaned up in an exitcall using unregister_pernet_operations:
> 
> WARNING: vmlinux.o(.text+0x868c34): Section mismatch in reference from the function nsim_devlink_exit() to the (unknown reference) .init.data:(unknown)
> The function nsim_devlink_exit() references
> the (unknown reference) __initdata (unknown).
> This is often because nsim_devlink_exit lacks a __initdata
> annotation or the annotation of (unknown) is wrong.
> WARNING: vmlinux.o(.text+0x868c64): Section mismatch in reference from the function nsim_devlink_init() to the (unknown reference) .init.data:(unknown)
> WARNING: vmlinux.o(.text+0x8692bc): Section mismatch in reference from the function nsim_fib_exit() to the (unknown reference) .init.data:(unknown)
> WARNING: vmlinux.o(.text+0x869300): Section mismatch in reference from the function nsim_fib_init() to the (unknown reference) .init.data:(unknown)
> 
> As that warning tells us, discarding the structure after a module is
> loaded would lead to a undefined behavior when that module is removed.
> 
> It might be possible to change that annotation so it has no effect for
> loadable modules, but I have not figured out exactly how to do that, and
> we want this to be fixed in -rc1.
> 
> This just removes the annotations, just like we do for all other such
> modules.
> 
> Fixes: 37923ed6b8ce ("netdevsim: Add simple FIB resource controller via devlink")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Hmmm...

__net_initdata defines to nothing if network namespaces are enabled.

That's the whole point.

Therefore, if NETNS is disabled, "unregister_pernet_operations" cannot
happen and this is the only time when __net_initdata actually does
something.

^ permalink raw reply

* Re: [PATCH 40/47] netfilter: nf_tables: build-in filter chain type
From: Arnd Bergmann @ 2018-04-04 15:50 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel, David Miller, Networking
In-Reply-To: <20180404154657.jjirlgjgdmzqg5hr@salvia>

On Wed, Apr 4, 2018 at 5:46 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Wed, Apr 04, 2018 at 05:38:31PM +0200, Arnd Bergmann wrote:
>> On Fri, Mar 30, 2018 at 1:46 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>> > One module per supported filter chain family type takes too much memory
>> > for very little code - too much modularization - place all chain filter
>> > definitions in one single file.
>> >
>> > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
>>
>> Hi Pablo,
>>
>> I've bisected a link error to this patch:
>>
>> net/netfilter/nft_reject_inet.o: In function `nft_reject_inet_eval':
>> nft_reject_inet.c:(.text+0xa7): undefined reference to `nf_send_unreach6'
>> nft_reject_inet.c:(.text+0x10c): undefined reference to `nf_send_unreach6'
>> nft_reject_inet.c:(.text+0x138): undefined reference to `nf_send_reset6'
>>
>> Unfortunately I don't immediately see what went wrong, maybe you
>> can spot it.
>
> Can you pass me your .config file? I will have a look at it. Thanks.
> Looks like some missing Kconfig stuff.

Sure, see https://pastebin.com/raw/yF9Y5Mmn

      Arnd

^ permalink raw reply

* Re: [Cluster-devel] [PATCH v2 0/2] gfs2: Stop using rhashtable_walk_peek
From: Herbert Xu @ 2018-04-04 15:48 UTC (permalink / raw)
  To: Bob Peterson
  Cc: Andreas Gruenbacher, cluster-devel, netdev, linux-kernel,
	NeilBrown, Thomas Graf, Tom Herbert
In-Reply-To: <1366548861.16037365.1522856788263.JavaMail.zimbra@redhat.com>

On Wed, Apr 04, 2018 at 11:46:28AM -0400, Bob Peterson wrote:
>
> The patches look good. The big question is whether to add them to this
> merge window while it's still open. Opinions?

We're still hashing out the rhashtable interface so I don't think
now is the time to rush things.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH] mac80211: Fix bad line warning
From: David Miller @ 2018-04-04 15:48 UTC (permalink / raw)
  To: standby24x7; +Cc: netdev, linux-kernel, linux-wireless
In-Reply-To: <20180404115333.1563-1-standby24x7@gmail.com>

From: Masanari Iida <standby24x7@gmail.com>
Date: Wed,  4 Apr 2018 20:53:33 +0900

> After 03fe2debbb2771fb90881e merged during 4.17 marge window,
> I start to see following warning during "make xmldocs"
> 
> ./include/net/mac80211.h:2083: warning: bad line:  >
> 
> Replace ">" with "*" fix the issue.
> 
> Signed-off-by: Masanari Iida <standby24x7@gmail.com>

Adding linux-wireless to CC:

> ---
>  include/net/mac80211.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> index d2279b2d61aa..b2f3a0c018e7 100644
> --- a/include/net/mac80211.h
> +++ b/include/net/mac80211.h
> @@ -2080,7 +2080,7 @@ struct ieee80211_txq {
>   *	virtual interface might not be given air time for the transmission of
>   *	the frame, as it is not synced with the AP/P2P GO yet, and thus the
>   *	deauthentication frame might not be transmitted.
> - >
> + *
>   * @IEEE80211_HW_DOESNT_SUPPORT_QOS_NDP: The driver (or firmware) doesn't
>   *	support QoS NDP for AP probing - that's most likely a driver bug.
>   *
> -- 
> 2.17.0.rc2.3.gc2a499e6c31e
> 

^ permalink raw reply

* Re: [PATCH] [net] nvmem: disallow modular CONFIG_NVMEM
From: David Miller @ 2018-04-04 15:48 UTC (permalink / raw)
  To: arnd
  Cc: srinivas.kandagatla, mike.looijmans, f.fainelli, andrew, netdev,
	gregkh, o.rempel, martin.blumenstingl, linux-kernel
In-Reply-To: <20180404103900.4090118-1-arnd@arndb.de>

From: Arnd Bergmann <arnd@arndb.de>
Date: Wed,  4 Apr 2018 12:38:40 +0200

> The new of_get_nvmem_mac_address() helper function causes a link error
> with CONFIG_NVMEM=m:
> 
> drivers/of/of_net.o: In function `of_get_nvmem_mac_address':
> of_net.c:(.text+0x168): undefined reference to `of_nvmem_cell_get'
> of_net.c:(.text+0x19c): undefined reference to `nvmem_cell_read'
> of_net.c:(.text+0x1a8): undefined reference to `nvmem_cell_put'
> 
> I could not come up with a good solution for this, as the code is always
> built-in. Using an #if IS_REACHABLE() check around it would solve the
> link time issue but then stop it from working in that configuration.
> Making of_nvmem_cell_get() an inline function could also solve that, but
> seems a bit ugly since it's somewhat larger than most inline functions,
> and it would just bring that problem into the callers.  Splitting the
> function into a separate file might be an alternative.
> 
> This uses the big hammer by making CONFIG_NVMEM itself a 'bool' symbol,
> which avoids the problem entirely but makes the vmlinux larger for anyone
> that might use NVMEM support but doesn't need it built-in otherwise.
> 
> Fixes: 9217e566bdee ("of_net: Implement of_get_nvmem_mac_address helper")
> Cc: Mike Looijmans <mike.looijmans@topic.nl>
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: netdev@vger.kernel.org
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> The problem arrived through the networking tree, but it's now in
> mainline, so the fix could go through either tree

Ok, applied, thanks Arnd.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox