Netdev List
 help / color / mirror / Atom feed
* [PATCH iproute2 v2 0/3] testsuite: make alltests fixes
From: Petr Vorel @ 2018-09-19 23:36 UTC (permalink / raw)
  To: netdev; +Cc: Petr Vorel, Stephen Hemminger, Phil Sutter, Luca Boccassi

Hi,

here are simply fixes to restore 'make alltests'.
Currently it does not run.

Kind regards,
Petr

Petr Vorel (3):
  testsuite: Fix missing generate_nlmsg
  testsuite: Generate generate_nlmsg when needed
  testsuite: Warn about empty $(IPVERS)

 testsuite/Makefile | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

-- 
2.19.0.rc2

^ permalink raw reply

* [PATCH iproute2 v2 1/3] testsuite: Fix missing generate_nlmsg
From: Petr Vorel @ 2018-09-19 23:36 UTC (permalink / raw)
  To: netdev; +Cc: Petr Vorel, Stephen Hemminger, Phil Sutter, Luca Boccassi
In-Reply-To: <20180919233624.18494-1-petr.vorel@gmail.com>

Commit ad23e152 caused generate_nlmsg to be always missing:

$ make alltests
make: ./tools/generate_nlmsg: Command not found

Create testclean: to remove only results directory.

Fixes: ad23e152 testsuite: remove all temp files and implement make clean

Signed-off-by: Petr Vorel <petr.vorel@gmail.com>
---
 testsuite/Makefile | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/testsuite/Makefile b/testsuite/Makefile
index d1ac997d..a31d4531 100644
--- a/testsuite/Makefile
+++ b/testsuite/Makefile
@@ -30,7 +30,6 @@ configure:
 
 compile: configure
 	echo "Entering iproute2" && cd iproute2 && $(MAKE) && cd ..;
-	$(MAKE) -C tools
 
 listtests:
 	@for t in $(TESTS); do \
@@ -39,9 +38,11 @@ listtests:
 
 alltests: $(TESTS)
 
-clean:
+testclean:
 	@echo "Removing $(RESULTS_DIR) dir ..."
 	@rm -rf $(RESULTS_DIR)
+
+clean: testclean
 	@rm -f iproute2/iproute2-this
 	@rm -f tests/ip/link/dev_wo_vf_rate.nl
 	$(MAKE) -C tools clean
@@ -49,18 +50,18 @@ clean:
 distclean: clean
 	echo "Entering iproute2" && cd iproute2 && $(MAKE) distclean && cd ..;
 
-$(TESTS): clean
+$(TESTS): testclean
 ifeq (,$(HAVE_UNSHARED_UTIL))
 	$(error Please install util-linux tools to run tests in separated network namespace)
 endif
 	@./tools/generate_nlmsg
 
 	@mkdir -p $(RESULTS_DIR)
-	
+
 	@for d in $(TESTS_DIR); do \
 	    mkdir -p $(RESULTS_DIR)/$$d; \
 	done
-	
+
 	@if [ "$(KCPATH)" = "/proc/config.gz" ]; then \
 		gunzip -c $(KCPATH) >$(KENVFN); \
 	elif [ "$(KCPATH)" != "" ]; then \
-- 
2.19.0.rc2

^ permalink raw reply related

* [PATCH iproute2 v2 2/3] testsuite: Generate generate_nlmsg when needed
From: Petr Vorel @ 2018-09-19 23:36 UTC (permalink / raw)
  To: netdev; +Cc: Petr Vorel, Stephen Hemminger, Phil Sutter, Luca Boccassi
In-Reply-To: <20180919233624.18494-1-petr.vorel@gmail.com>

Commit 886f2c43 added generate_nlmsg.c. Running alltests
target, which uses the binary required to run 'make -C tools' before.

Fixes: 886f2c43 testsuite: Generate nlmsg blob at runtime

Signed-off-by: Petr Vorel <petr.vorel@gmail.com>
---
 testsuite/Makefile | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/testsuite/Makefile b/testsuite/Makefile
index a31d4531..1c2467f5 100644
--- a/testsuite/Makefile
+++ b/testsuite/Makefile
@@ -28,7 +28,7 @@ endif
 configure:
 	echo "Entering iproute2" && cd iproute2 && $(MAKE) configure && cd ..;
 
-compile: configure
+compile: configure generate_nlmsg
 	echo "Entering iproute2" && cd iproute2 && $(MAKE) && cd ..;
 
 listtests:
@@ -36,7 +36,10 @@ listtests:
 		echo "$$t"; \
 	done
 
-alltests: $(TESTS)
+generate_nlmsg:
+	$(MAKE) -C tools
+
+alltests: generate_nlmsg $(TESTS)
 
 testclean:
 	@echo "Removing $(RESULTS_DIR) dir ..."
-- 
2.19.0.rc2

^ permalink raw reply related

* [PATCH iproute2 v2 3/3] testsuite: Warn about empty $(IPVERS)
From: Petr Vorel @ 2018-09-19 23:36 UTC (permalink / raw)
  To: netdev; +Cc: Petr Vorel, Stephen Hemminger, Phil Sutter, Luca Boccassi
In-Reply-To: <20180919233624.18494-1-petr.vorel@gmail.com>

alltests target requires having symlink created by configure target
(default target). Without that there is no test being run.

Signed-off-by: Petr Vorel <petr.vorel@gmail.com>
---
 testsuite/Makefile | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/testsuite/Makefile b/testsuite/Makefile
index 1c2467f5..b3aebec1 100644
--- a/testsuite/Makefile
+++ b/testsuite/Makefile
@@ -54,6 +54,9 @@ distclean: clean
 	echo "Entering iproute2" && cd iproute2 && $(MAKE) distclean && cd ..;
 
 $(TESTS): testclean
+ifeq (,$(IPVERS))
+	$(error Please run make first)
+endif
 ifeq (,$(HAVE_UNSHARED_UTIL))
 	$(error Please install util-linux tools to run tests in separated network namespace)
 endif
-- 
2.19.0.rc2

^ permalink raw reply related

* [Patch net-next v3] net_sched: change tcf_del_walker() to take idrinfo->lock
From: Cong Wang @ 2018-09-19 23:37 UTC (permalink / raw)
  To: netdev; +Cc: jiri, jhs, Vlad Buslov, Cong Wang

From: Vlad Buslov <vladbu@mellanox.com>

From: Vlad Buslov <vladbu@mellanox.com>

Action API was changed to work with actions and action_idr in concurrency
safe manner, however tcf_del_walker() still uses actions without taking a
reference or idrinfo->lock first, and deletes them directly, disregarding
possible concurrent delete.

Change tcf_del_walker() to take idrinfo->lock while iterating over actions
and use new tcf_idr_release_unsafe() to release them while holding the
lock.

And the blocking function fl_hw_destroy_tmplt() could be called when we
put a filter chain, so defer it to a work queue.

Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
[xiyou.wangcong@gmail.com: heavily modify the code and changelog]
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 net/sched/act_api.c    | 20 +++++++++++++++++++-
 net/sched/cls_flower.c | 13 +++++++++++--
 2 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 6f118d62c731..fac8c769454f 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -246,6 +246,20 @@ static int tcf_dump_walker(struct tcf_idrinfo *idrinfo, struct sk_buff *skb,
 	goto done;
 }
 
+static int tcf_idr_release_unsafe(struct tc_action *p)
+{
+	if (atomic_read(&p->tcfa_bindcnt) > 0)
+		return -EPERM;
+
+	if (refcount_dec_and_test(&p->tcfa_refcnt)) {
+		idr_remove(&p->idrinfo->action_idr, p->tcfa_index);
+		tcf_action_cleanup(p);
+		return ACT_P_DELETED;
+	}
+
+	return 0;
+}
+
 static int tcf_del_walker(struct tcf_idrinfo *idrinfo, struct sk_buff *skb,
 			  const struct tc_action_ops *ops)
 {
@@ -262,15 +276,19 @@ static int tcf_del_walker(struct tcf_idrinfo *idrinfo, struct sk_buff *skb,
 	if (nla_put_string(skb, TCA_KIND, ops->kind))
 		goto nla_put_failure;
 
+	spin_lock(&idrinfo->lock);
 	idr_for_each_entry_ul(idr, p, id) {
-		ret = __tcf_idr_release(p, false, true);
+		ret = tcf_idr_release_unsafe(p);
 		if (ret == ACT_P_DELETED) {
 			module_put(ops->owner);
 			n_i++;
 		} else if (ret < 0) {
+			spin_unlock(&idrinfo->lock);
 			goto nla_put_failure;
 		}
 	}
+	spin_unlock(&idrinfo->lock);
+
 	if (nla_put_u32(skb, TCA_FCNT, n_i))
 		goto nla_put_failure;
 	nla_nest_end(skb, nest);
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 4b8dd37dd4f8..0ed6630a5049 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -79,6 +79,7 @@ struct fl_flow_tmplt {
 	struct fl_flow_key mask;
 	struct flow_dissector dissector;
 	struct tcf_chain *chain;
+	struct rcu_work rwork;
 };
 
 struct cls_fl_head {
@@ -1437,14 +1438,22 @@ static void *fl_tmplt_create(struct net *net, struct tcf_chain *chain,
 	return ERR_PTR(err);
 }
 
-static void fl_tmplt_destroy(void *tmplt_priv)
+static void fl_tmplt_destroy_work(struct work_struct *work)
 {
-	struct fl_flow_tmplt *tmplt = tmplt_priv;
+	struct fl_flow_tmplt *tmplt = container_of(to_rcu_work(work),
+						 struct fl_flow_tmplt, rwork);
 
 	fl_hw_destroy_tmplt(tmplt->chain, tmplt);
 	kfree(tmplt);
 }
 
+static void fl_tmplt_destroy(void *tmplt_priv)
+{
+	struct fl_flow_tmplt *tmplt = tmplt_priv;
+
+	tcf_queue_work(&tmplt->rwork, fl_tmplt_destroy_work);
+}
+
 static int fl_dump_key_val(struct sk_buff *skb,
 			   void *val, int val_type,
 			   void *mask, int mask_type, int len)
-- 
2.14.4

^ permalink raw reply related

* Re: [PATCH bpf-next 2/3] bpf: emit RECORD_MMAP events for bpf prog load/unload
From: Song Liu @ 2018-09-19 23:44 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: David S . Miller, daniel@iogearbox.net, peterz@infradead.org,
	acme@kernel.org, netdev@vger.kernel.org, Kernel Team
In-Reply-To: <20180919223935.999270-3-ast@kernel.org>



> On Sep 19, 2018, at 3:39 PM, Alexei Starovoitov <ast@kernel.org> wrote:
> 
> use perf_event_mmap_bpf_prog() helper to notify user space
> about JITed bpf programs.
> Use RECORD_MMAP perf event to tell user space where JITed bpf program was loaded.
> Use empty program name as unload indication.
> 
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> ---
> kernel/bpf/core.c | 22 ++++++++++++++++++++--
> 1 file changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> index 3f5bf1af0826..ddf11fdafd36 100644
> --- a/kernel/bpf/core.c
> +++ b/kernel/bpf/core.c
> @@ -384,7 +384,7 @@ bpf_get_prog_addr_region(const struct bpf_prog *prog,
> 	*symbol_end   = addr + hdr->pages * PAGE_SIZE;
> }
> 
> -static void bpf_get_prog_name(const struct bpf_prog *prog, char *sym)
> +static char *bpf_get_prog_name(const struct bpf_prog *prog, char *sym)
> {
> 	const char *end = sym + KSYM_NAME_LEN;
> 
> @@ -402,9 +402,10 @@ static void bpf_get_prog_name(const struct bpf_prog *prog, char *sym)
> 	sym += snprintf(sym, KSYM_NAME_LEN, "bpf_prog_");
> 	sym  = bin2hex(sym, prog->tag, sizeof(prog->tag));
> 	if (prog->aux->name[0])
> -		snprintf(sym, (size_t)(end - sym), "_%s", prog->aux->name);
> +		sym += snprintf(sym, (size_t)(end - sym), "_%s", prog->aux->name);
> 	else
> 		*sym = 0;
> +	return sym;
> }
> 
> static __always_inline unsigned long
> @@ -480,23 +481,40 @@ static bool bpf_prog_kallsyms_verify_off(const struct bpf_prog *fp)
> 
> void bpf_prog_kallsyms_add(struct bpf_prog *fp)
> {
> +	unsigned long symbol_start, symbol_end;
> +	char buf[KSYM_NAME_LEN], *sym;
> +
> 	if (!bpf_prog_kallsyms_candidate(fp) ||
> 	    !capable(CAP_SYS_ADMIN))
> 		return;
> 
> +	bpf_get_prog_addr_region(fp, &symbol_start, &symbol_end);
> +	sym = bpf_get_prog_name(fp, buf);
> +	sym++; /* sym - buf is the length of the name including trailing 0 */
> +	while (!IS_ALIGNED(sym - buf, sizeof(u64)))
> +		*sym++ = 0;

nit: This logic feels a little weird to me. How about we wrap the extra logic
in a separate function:

size_t bpf_get_prog_name_u64_aligned(const struct bpf_prog fp, char *buf)

where the return value is the u64 aligned size. 

Other than this 

Acked-by: Song Liu <songliubraving@fb.com>

> 	spin_lock_bh(&bpf_lock);
> 	bpf_prog_ksym_node_add(fp->aux);
> 	spin_unlock_bh(&bpf_lock);
> +	perf_event_mmap_bpf_prog(symbol_start, symbol_end - symbol_start,
> +				 buf, sym - buf);
> }
> 
> void bpf_prog_kallsyms_del(struct bpf_prog *fp)
> {
> +	unsigned long symbol_start, symbol_end;
> +	/* mmap_record.filename cannot be NULL and has to be u64 aligned */
> +	char buf[sizeof(u64)] = {};
> +
> 	if (!bpf_prog_kallsyms_candidate(fp))
> 		return;
> 
> 	spin_lock_bh(&bpf_lock);
> 	bpf_prog_ksym_node_del(fp->aux);
> 	spin_unlock_bh(&bpf_lock);
> +	bpf_get_prog_addr_region(fp, &symbol_start, &symbol_end);
> +	perf_event_mmap_bpf_prog(symbol_start, symbol_end - symbol_start,
> +				 buf, sizeof(buf));
> }
> 
> static struct bpf_prog *bpf_prog_kallsyms_find(unsigned long addr)
> -- 
> 2.17.1
> 

^ permalink raw reply

* Re: [PATCH net-next 1/2] net: linkwatch: add check for netdevice being present to linkwatch_do_dev
From: Florian Fainelli @ 2018-09-20  0:03 UTC (permalink / raw)
  To: Heiner Kallweit, Andrew Lunn, David Miller
  Cc: netdev@vger.kernel.org, Geert Uytterhoeven
In-Reply-To: <11beeaa9-57d5-e641-9486-f2ba202d0998@gmail.com>

On 09/18/2018 12:55 PM, Heiner Kallweit wrote:
> When bringing down the netdevice (incl. detaching it) and calling
> netif_carrier_off directly or indirectly the latter triggers an
> asynchronous linkwatch event.
> This linkwatch event eventually may fail to access chip registers in
> the ndo_get_stats/ndo_get_stats64 callback because the device isn't
> accessible any longer, see call trace in [0].
> 
> To prevent this scenario don't check for IFF_UP only, but also make
> sure that the netdevice is present.
> 
> [0] https://lists.openwall.net/netdev/2018/03/15/62
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>

Thanks Heiner!
-- 
Florian

^ permalink raw reply

* Re: [PATCH net-next 2/2] net: phy: call state machine synchronously in phy_stop
From: Florian Fainelli @ 2018-09-20  0:03 UTC (permalink / raw)
  To: Heiner Kallweit, Andrew Lunn, David Miller
  Cc: netdev@vger.kernel.org, Geert Uytterhoeven
In-Reply-To: <a58e1448-0a10-b7a1-ccca-ca99f146c8ea@gmail.com>

On 09/18/2018 12:56 PM, Heiner Kallweit wrote:
> phy_stop() may be called e.g. when suspending, therefore all needed
> actions should be performed synchronously. Therefore add a synchronous
> call to the state machine.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>

Yes!
-- 
Florian

^ permalink raw reply

* Re: [PATCH] net: phy: don't reschedule state machine when PHY is halted
From: Florian Fainelli @ 2018-09-20  0:04 UTC (permalink / raw)
  To: Heiner Kallweit, Andrew Lunn, David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <df6d5770-7508-46f8-106f-6d00f8e1aac8@gmail.com>

On 09/18/2018 01:17 PM, Heiner Kallweit wrote:
> On 18.09.2018 22:02, Florian Fainelli wrote:
>> On 09/18/2018 12:12 PM, Heiner Kallweit wrote:
>>> I think I've seen a similar or same patch before, not sure why it
>>> didn't make it yet. When being in state PHY_HALTED we don't have to
>>> reschedule the state machine, phy_start() will start it again.
>>
>> Yes, this is conceptually the same patch as as this one:
>>
>> https://patchwork.ozlabs.org/patch/830415/
>>
> Thanks for the link, this is what I was referring to.
> 
>> I prefer your version, though the comment in the original patch explains
>> why.
>>
> To be sure I understand you correctly:
> You're fine with the patch as is or would you prefer to add a comment
> like in the original patch ?

Correct, unless you think this does not warrant a comment and it is
clear enough as-is?

> 
>>>
>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>>> ---
>>>  drivers/net/phy/phy.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
>>> index 1ee25877c..c78203b25 100644
>>> --- a/drivers/net/phy/phy.c
>>> +++ b/drivers/net/phy/phy.c
>>> @@ -1123,7 +1123,7 @@ void phy_state_machine(struct work_struct *work)
>>>  	 * PHY, if PHY_IGNORE_INTERRUPT is set, then we will be moving
>>>  	 * between states from phy_mac_interrupt()
>>>  	 */
>>> -	if (phy_polling_mode(phydev))
>>> +	if (phy_polling_mode(phydev) && old_state != PHY_HALTED)
>>>  		queue_delayed_work(system_power_efficient_wq, &phydev->state_queue,
>>>  				   PHY_STATE_TIME * HZ);
>>>  }
>>>
>>
>>
> 


-- 
Florian

^ permalink raw reply

* [PATCH net] af_key: free SKBs under RCU protection
From: Sean Tranchetti @ 2018-09-20  0:18 UTC (permalink / raw)
  To: netdev, steffen.klassert; +Cc: Sean Tranchetti

pfkey_broadcast() can make calls to pfkey_broadcast_one() which
will clone or copy the passed in SKB. This new SKB will be assigned
the sock_rfree() function as its destructor, which requires that
the socket reference the SKB contains is valid when the SKB is freed.

If this SKB is ever passed to pfkey_broadcast() again by some other
function (such as pkfey_dump() or pfkey_promisc) it will then be
freed there. However, since this free occurs outside of RCU protection,
it is possible that userspace could close the socket and trigger
pfkey_release() to free the socket before sock_rfree() can run, creating
the following race condition:

1: An SKB belonging to the pfkey socket is passed to pfkey_broadcast().
   It performs the broadcast to any other sockets, and calls
   rcu_read_unlock(), but does not yet reach kfree_skb().
2: Userspace closes the socket, triggering pfkey_realse(). Since no one
   holds the RCU lock, synchronize_rcu() returns and it is allowed to
   continue. It calls sock_put() to free the socket.
3: pfkey_broadcast() now calls kfree_skb() on the original SKB it was
   passed, triggering a call to sock_rfree(). This function now accesses
   the freed struct sock * via skb->sk, and attempts to update invalid
   memory.

By ensuring that the pfkey_broadcast() also frees the SKBs while it holds
the RCU lock, we can ensure that the socket will remain valid when the SKB
is freed, avoiding crashes like the following:

Unable to handle kernel paging request at virtual address 6b6b6b6b6b6c4b
[006b6b6b6b6b6c4b] address between user and kernel address ranges
Internal error: Oops: 96000004 [#1] PREEMPT SMP
task: fffffff78f65b380 task.stack: ffffff8049a88000
pc : sock_rfree+0x38/0x6c
lr : skb_release_head_state+0x6c/0xcc
Process repro (pid: 7117, stack limit = 0xffffff8049a88000)
Call trace:
	sock_rfree+0x38/0x6c
	skb_release_head_state+0x6c/0xcc
	skb_release_all+0x1c/0x38
	__kfree_skb+0x1c/0x30
	kfree_skb+0xd0/0xf4
	pfkey_broadcast+0x14c/0x18c
	pfkey_sendmsg+0x1d8/0x408
	sock_sendmsg+0x44/0x60
	___sys_sendmsg+0x1d0/0x2a8
	__sys_sendmsg+0x64/0xb4
	SyS_sendmsg+0x34/0x4c
	el0_svc_naked+0x34/0x38
Kernel panic - not syncing: Fatal exception

Signed-off-by: Sean Tranchetti <stranche@codeaurora.org>
---
 net/key/af_key.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/key/af_key.c b/net/key/af_key.c
index 9d61266..dd257c7 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -275,13 +275,13 @@ static int pfkey_broadcast(struct sk_buff *skb, gfp_t allocation,
 		if ((broadcast_flags & BROADCAST_REGISTERED) && err)
 			err = err2;
 	}
-	rcu_read_unlock();
 
 	if (one_sk != NULL)
 		err = pfkey_broadcast_one(skb, &skb2, allocation, one_sk);
 
 	kfree_skb(skb2);
 	kfree_skb(skb);
+	rcu_read_unlock();
 	return err;
 }
 
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH] net: toshiba: remove a redundant local variable 'index_specified'
From: David Miller @ 2018-09-20  6:09 UTC (permalink / raw)
  To: zhongjiang; +Cc: benh, paulus, mpe, netdev, linux-kernel
In-Reply-To: <1537408565-22315-1-git-send-email-zhongjiang@huawei.com>

From: zhong jiang <zhongjiang@huawei.com>
Date: Thu, 20 Sep 2018 09:56:05 +0800

> The local variable 'index_specified' is never used after being assigned.
> hence it should be redundant adn can be removed.
> 
> Signed-off-by: zhong jiang <zhongjiang@huawei.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next] e1000: remove set but not used variable 'txb2b'
From: David Miller @ 2018-09-20  6:09 UTC (permalink / raw)
  To: yuehaibing
  Cc: jeffrey.t.kirsher, intel-wired-lan, netdev, kernel-janitors,
	linux-kernel
In-Reply-To: <1537409078-2374-1-git-send-email-yuehaibing@huawei.com>

From: YueHaibing <yuehaibing@huawei.com>
Date: Thu, 20 Sep 2018 02:04:38 +0000

> Fixes gcc '-Wunused-but-set-variable' warning:
> 
> drivers/net/ethernet/intel/e1000/e1000_main.c: In function 'e1000_watchdog':
> drivers/net/ethernet/intel/e1000/e1000_main.c:2436:9: warning:
>  variable 'txb2b' set but not used [-Wunused-but-set-variable]
> 
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH net-next] atl1: remove set but not used variable 'advertising'
From: David Miller @ 2018-09-20  6:10 UTC (permalink / raw)
  To: yuehaibing
  Cc: jcliburn, chris.snook, keescook, zhongjiang, netdev,
	kernel-janitors, linux-kernel
In-Reply-To: <1537411290-5132-1-git-send-email-yuehaibing@huawei.com>

From: YueHaibing <yuehaibing@huawei.com>
Date: Thu, 20 Sep 2018 02:41:30 +0000

> Fixes gcc '-Wunused-but-set-variable' warning:
> 
> drivers/net/ethernet/atheros/atlx/atl1.c: In function 'atl1_set_link_ksettings':
> drivers/net/ethernet/atheros/atlx/atl1.c:3280:6: warning:
>  variable 'advertising' set but not used [-Wunused-but-set-variable]
> 
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>

Applied.

^ permalink raw reply

* Re: [PATCH] isdn/hisax: Remove unnecessary parenthesis
From: David Miller @ 2018-09-20  6:11 UTC (permalink / raw)
  To: natechancellor; +Cc: isdn, netdev, linux-kernel, ndesaulniers
In-Reply-To: <20180920040523.21671-1-natechancellor@gmail.com>

From: Nathan Chancellor <natechancellor@gmail.com>
Date: Wed, 19 Sep 2018 21:05:23 -0700

> Clang warns when more than one set of parentheses are used for single
> conditional statements:
> 
> drivers/isdn/hisax/w6692.c:627:30: warning: equality comparison with
> extraneous parentheses [-Wparentheses-equality]
>                 if ((cs->dc.w6692.ph_state == W_L1IND_DRD)) {
>                      ~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~
> drivers/isdn/hisax/w6692.c:627:30: note: remove extraneous parentheses
> around the comparison to silence this warning
>                 if ((cs->dc.w6692.ph_state == W_L1IND_DRD)) {
>                     ~                      ^             ~
> drivers/isdn/hisax/w6692.c:627:30: note: use '=' to turn this equality
> comparison into an assignment
>                 if ((cs->dc.w6692.ph_state == W_L1IND_DRD)) {
>                                            ^~
>                                            =
> 1 warning generated.
> 
> Remove the parentheses to silence this warning.
> 
> Reported-by: Nick Desaulniers <ndesaulniers@google.com>
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>

Applied.

^ permalink raw reply

* Re: [PATCH] net: macb: Clean 64b dma addresses if they are not detected
From: Michal Simek @ 2018-09-20  6:23 UTC (permalink / raw)
  To: Edgar E. Iglesias, Michal Simek
  Cc: linux-kernel, monstr, Edgar E. Iglesias, David S. Miller, netdev,
	u-boot, Joe Hershberger, Nicolas Ferre
In-Reply-To: <20180919180806.3tps6yukhm3ry43i@toto>

On 19.9.2018 20:08, Edgar E. Iglesias wrote:
> On Wed, Sep 19, 2018 at 06:08:18PM +0200, Michal Simek wrote:
>> Clear ADDR64 dma bit in DMACFG register in case that HW_DMA_CAP_64B
>> is not detected on 64bit system.
>> The issue was observed when bootloader(u-boot) does not check macb
>> feature at DCFG6 register (DAW64_OFFSET) and enabling 64bit dma support
>> by default. Then macb driver is reading DMACFG register back and only
>> adding 64bit dma configuration but not cleaning it out.
>>
>> This is also align with other features which are also cleared if they are not
>> present.
> 
> Hi Michal,
> 
>>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> ---
>>
>>  drivers/net/ethernet/cadence/macb_main.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
>> index 16e4ef7d7185..79707dff3f13 100644
>> --- a/drivers/net/ethernet/cadence/macb_main.c
>> +++ b/drivers/net/ethernet/cadence/macb_main.c
>> @@ -2163,6 +2163,8 @@ static void macb_configure_dma(struct macb *bp)
>>  #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
>>  		if (bp->hw_dma_cap & HW_DMA_CAP_64B)
>>  			dmacfg |= GEM_BIT(ADDR64);
>> +		else
>> +			dmacfg &= ~GEM_BIT(ADDR64);
>>  #endif
> 
> I think you might want to do this clearing outside of the #ifdef.
> If CONFIG_ARCH_DMA_ADDR_T_64BIT is not defined, we'd want to make
> sure the ADDR64 is cleared. E.g something like:
> 
>              dmacfg &= ~GEM_BIT(ADDR64);
> #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
>              if (bp->hw_dma_cap & HW_DMA_CAP_64B)
>                      dmacfg |= GEM_BIT(ADDR64);
> #endif
> 
> 
> Same thing for the USE_HWSTAMP/PTP flags below.

Origin patch, which introduce this read with mask,
macfg = gem_readl(bp, DMACFG) & ~GEM_BF(RXBS, -1L);
was done in 2011 and from that time this function was extended a little
bit. I am even not quite sure if make sense to read this reg and apply
setting on the top of it.

Nicolas: Isn't it better simply compose that reg from scratch?

Thanks,
Michal

^ permalink raw reply

* Re: [PATCH bpf-next 1/3] perf/core: introduce perf_event_mmap_bpf_prog
From: Alexei Starovoitov @ 2018-09-20  0:53 UTC (permalink / raw)
  To: Song Liu, Alexei Starovoitov
  Cc: David S . Miller, daniel@iogearbox.net, peterz@infradead.org,
	acme@kernel.org, netdev@vger.kernel.org, Kernel Team
In-Reply-To: <4DBB524C-09B1-46BD-A56E-1E372E87F2E8@fb.com>

On 9/19/18 4:30 PM, Song Liu wrote:
>
>
>> On Sep 19, 2018, at 3:39 PM, Alexei Starovoitov <ast@kernel.org> wrote:
>>
>> introduce perf_event_mmap_bpf_prog() helper to emit RECORD_MMAP events
>> into perf ring buffer.
>> It's used by bpf load/unload logic to notify user space of addresses
>> and names of JITed bpf programs.
>>
>> Note that event->mmap.pid == -1 is an existing indicator of kernel event.
>> In addition use event->mmap.tid == BPF_FS_MAGIC to indicate bpf related
>> RECORD_MMAP event.
>>
>> Alternatively it's possible to introduce new 'enum perf_event_type' command
>> specificially for bpf prog load/unload, but existing RECORD_MMAP
>> is very close, so the choice made by this patch is to extend it.
>>
>> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
>
> Acked-by: Song Liu <songliubraving@fb.com>
>
> I guess we should also use this for kernel modules load/unload?

yes. that's possible.
There is similar issue with modules today that get unloaded
before 'perf report'.
Synthetic record_mmap for modules that perf emits into perf.data
has filename==module_name. It solves typical use case though.
We can extend record_mmap further and let kernel emit them
for module load/unload.
Some new magic for 'tid' would be necessary.

^ permalink raw reply

* Re: [PATCH bpf-next 2/3] bpf: emit RECORD_MMAP events for bpf prog load/unload
From: Alexei Starovoitov @ 2018-09-20  0:59 UTC (permalink / raw)
  To: Song Liu, Alexei Starovoitov
  Cc: David S . Miller, daniel@iogearbox.net, peterz@infradead.org,
	acme@kernel.org, netdev@vger.kernel.org, Kernel Team
In-Reply-To: <3FB6EC86-700C-4C80-A467-55B6753B07FE@fb.com>

On 9/19/18 4:44 PM, Song Liu wrote:
>
>
>> On Sep 19, 2018, at 3:39 PM, Alexei Starovoitov <ast@kernel.org> wrote:
>>
>> use perf_event_mmap_bpf_prog() helper to notify user space
>> about JITed bpf programs.
>> Use RECORD_MMAP perf event to tell user space where JITed bpf program was loaded.
>> Use empty program name as unload indication.
>>
>> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
>> ---
>> kernel/bpf/core.c | 22 ++++++++++++++++++++--
>> 1 file changed, 20 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
>> index 3f5bf1af0826..ddf11fdafd36 100644
>> --- a/kernel/bpf/core.c
>> +++ b/kernel/bpf/core.c
>> @@ -384,7 +384,7 @@ bpf_get_prog_addr_region(const struct bpf_prog *prog,
>> 	*symbol_end   = addr + hdr->pages * PAGE_SIZE;
>> }
>>
>> -static void bpf_get_prog_name(const struct bpf_prog *prog, char *sym)
>> +static char *bpf_get_prog_name(const struct bpf_prog *prog, char *sym)
>> {
>> 	const char *end = sym + KSYM_NAME_LEN;
>>
>> @@ -402,9 +402,10 @@ static void bpf_get_prog_name(const struct bpf_prog *prog, char *sym)
>> 	sym += snprintf(sym, KSYM_NAME_LEN, "bpf_prog_");
>> 	sym  = bin2hex(sym, prog->tag, sizeof(prog->tag));
>> 	if (prog->aux->name[0])
>> -		snprintf(sym, (size_t)(end - sym), "_%s", prog->aux->name);
>> +		sym += snprintf(sym, (size_t)(end - sym), "_%s", prog->aux->name);
>> 	else
>> 		*sym = 0;
>> +	return sym;
>> }
>>
>> static __always_inline unsigned long
>> @@ -480,23 +481,40 @@ static bool bpf_prog_kallsyms_verify_off(const struct bpf_prog *fp)
>>
>> void bpf_prog_kallsyms_add(struct bpf_prog *fp)
>> {
>> +	unsigned long symbol_start, symbol_end;
>> +	char buf[KSYM_NAME_LEN], *sym;
>> +
>> 	if (!bpf_prog_kallsyms_candidate(fp) ||
>> 	    !capable(CAP_SYS_ADMIN))
>> 		return;
>>
>> +	bpf_get_prog_addr_region(fp, &symbol_start, &symbol_end);
>> +	sym = bpf_get_prog_name(fp, buf);
>> +	sym++; /* sym - buf is the length of the name including trailing 0 */
>> +	while (!IS_ALIGNED(sym - buf, sizeof(u64)))
>> +		*sym++ = 0;
>
> nit: This logic feels a little weird to me. How about we wrap the extra logic
> in a separate function:
>
> size_t bpf_get_prog_name_u64_aligned(const struct bpf_prog fp, char *buf)

probably bpf_get_prog_name_u64_padded() ?
would be cleaner indeed.

Will send v2 once Peter and Arnaldo provide their feedback.

Thanks for reviewing!

^ permalink raw reply

* Re: [PATCH v2 2/4] dt-bindings: net: qcom: Add binding for shared mdio bus
From: Wang, Dongsheng @ 2018-09-20  1:33 UTC (permalink / raw)
  To: Timur Tabi, Andrew Lunn
  Cc: Florian Fainelli, davem@davemloft.net, Zheng, Joey,
	netdev@vger.kernel.org, devicetree@vger.kernel.org
In-Reply-To: <1226d3d3-e2ba-571d-5b70-b246cbde2fb6@kernel.org>

On 2018/9/19 22:15, Timur Tabi wrote:
> On 9/19/18 7:25 AM, Andrew Lunn wrote:
>> ACPI is completely separate and should not affect the DT binding.
>> I've not yet looked at the ACPI changes you added.
> Just FYI, there is no device tree platform on which the upstream EMAC 
> driver works.  All of the DT code in the driver is theoretical.  It 
> worked once on a prototype platform, when I originally wrote the code, 
> but since then DT support is mostly a guess.
>
> The focus of any patches for the EMAC should be ACPI, not DT.  If 
> anything, ACPI support should come first.  No one should be writing or 
> reviewing DT code before ACPI code.
>
> The upstream EMAC driver is only known to work on the QDF2400, which is 
> an ACPI-only chip.  I feel like I've been repeating this too often.
>
Ok, I just focus on ACPI, and keep DT code no changes.

Cheers,
Dongsheng

^ permalink raw reply

* Re: [PATCH] netfilter: nf_tables: add SECMARK support
From: Christian Göttsche @ 2018-09-20  7:18 UTC (permalink / raw)
  To: casey
  Cc: pablo, kadlec, fw, davem, netfilter-devel, coreteam, netdev,
	linux-kernel, Paul Moore, Stephen Smalley, Eric Paris, jmorris,
	serge, selinux, linux-security-module
In-Reply-To: <75b3fed9-e549-4ed0-c435-ec4795fc1e39@schaufler-ca.com>

> I've only had a cursory look at your patch, but how is it
> different from what's in xt_SECMARK.c ?

xt_SEXMARK.c is for xtables, use-able in iptables; this is for nftables (nft)

^ permalink raw reply

* [PATCH] net: netronome: remove redundant continue
From: zhong jiang @ 2018-09-20  8:02 UTC (permalink / raw)
  To: davem; +Cc: dirk.vandermerwe, simon.horman, netdev, linux-kernel

The continue will not truely skip any code. hence it is safe to
remove it.

Signed-off-by: zhong jiang <zhongjiang@huawei.com>
---
 drivers/net/ethernet/netronome/nfp/nfp_net_main.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_main.c b/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
index 0b1ac9c..50d7b58 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
@@ -230,10 +230,8 @@ static void nfp_net_pf_free_vnics(struct nfp_pf *pf)
 		ctrl_bar += NFP_PF_CSR_SLICE_SIZE;
 
 		/* Kill the vNIC if app init marked it as invalid */
-		if (nn->port && nn->port->type == NFP_PORT_INVALID) {
+		if (nn->port && nn->port->type == NFP_PORT_INVALID)
 			nfp_net_pf_free_vnic(pf, nn);
-			continue;
-		}
 	}
 
 	if (list_empty(&pf->vnics))
-- 
1.7.12.4

^ permalink raw reply related

* Re: [PATCH 00/22] various dynamic_debug patches
From: Rafael J. Wysocki @ 2018-09-20  8:05 UTC (permalink / raw)
  To: Rasmus Villemoes
  Cc: Jason Baron, Andrew Morton, linux-kernel, Greg Kroah-Hartman,
	netdev, Petr Mladek, Steven Rostedt, linux-btrfs, linux-acpi, x86
In-Reply-To: <20180919220444.23190-1-linux@rasmusvillemoes.dk>

On Thursday, September 20, 2018 12:04:22 AM CEST Rasmus Villemoes wrote:
> This started as an experiment to see how hard it would be to change
> the four pointers in struct _ddebug into relative offsets, a la
> CONFIG_GENERIC_BUG_RELATIVE_POINTERS, thus saving 16 bytes per
> pr_debug site (and thus exactly making up for the extra space used by
> the introduction of jump labels in 9049fc74). I stumbled on a few
> things that are probably worth fixing regardless of whether the latter
> half of this series is deemed worthwhile.
> 
> Patch relationships: 1-2, 3-4, 5-6 and 15-16 can be applied
> individually, though 2, 4 and 6 probably makes most sense in the
> context of the final goal of the series.

OK, I can take the [15-16/22] separately.

Thanks,
Rafael

^ permalink raw reply

* [PATCH] net: amd: remove redundant continue
From: zhong jiang @ 2018-09-20  8:07 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux-kernel

The continue will not truely skip any code. hence it is safe to
remove it.

Signed-off-by: zhong jiang <zhongjiang@huawei.com>
---
 drivers/net/ethernet/amd/ni65.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/amd/ni65.c b/drivers/net/ethernet/amd/ni65.c
index e248d1a..8931ce6 100644
--- a/drivers/net/ethernet/amd/ni65.c
+++ b/drivers/net/ethernet/amd/ni65.c
@@ -435,10 +435,8 @@ static int __init ni65_probe1(struct net_device *dev,int ioaddr)
 		}
 		if(cards[i].vendor_id) {
 			for(j=0;j<3;j++)
-				if(inb(ioaddr+cards[i].addr_offset+j) != cards[i].vendor_id[j]) {
+				if(inb(ioaddr+cards[i].addr_offset+j) != cards[i].vendor_id[j])
 					release_region(ioaddr, cards[i].total_size);
-					continue;
-			  }
 		}
 		break;
 	}
-- 
1.7.12.4

^ permalink raw reply related

* Re: [PATCH 00/11] pull request for net: batman-adv 2018-09-19
From: Simon Wunderlich @ 2018-09-20  8:09 UTC (permalink / raw)
  To: David Miller
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	b.a.t.m.a.n-ZwoEplunGu2X36UT3dwllkB+6BGkLq7r
In-Reply-To: <20180919.203434.1784595444537176012.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>

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

On Wednesday, September 19, 2018 8:34:34 PM CEST David Miller wrote:
> From: Simon Wunderlich <sw-2YrNx6rUIHYiY0qSoAWiAoQuADTiUCJX@public.gmane.org>
> Date: Wed, 19 Sep 2018 14:22:02 +0200
> 
> > here are some bugfixes which we would like to see integrated into net.
> > 
> > We forgot to bump the version number in the last round for net-next, so
> > the belated patch to do that is included - we hope you can adopt it.
> > This will most likely create a merge conflict later when merging into
> > net-next with this rounds net-next patchset, but net-next should keep
> > the 2018.4 version[1].
> > 
> > Please pull or let me know of any problem!
> 
> Pulled, thanks Simon.

Great, thanks!

> 
> In the future, you can put that merge resolution guidance into the
> merge commit message.
> 
> When people give annotations like this, I refer to them when I do
> merges and it helps me a lot.  If it's in the actual GIT history,
> that is tons easier than if it just appeared in your pull request
> email.
> 
> I manually included it in the merge commit message this time.
> 
> Thanks!

OK, thanks for the hint! We will do this.

Cheers,
     Simon

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* [PATCH net-next] atl1: remove set but not used variable 'advertising'
From: YueHaibing @ 2018-09-20  2:41 UTC (permalink / raw)
  To: Jay Cliburn, Chris Snook, David S. Miller, Kees Cook, zhong jiang
  Cc: YueHaibing, netdev, kernel-janitors, linux-kernel

Fixes gcc '-Wunused-but-set-variable' warning:

drivers/net/ethernet/atheros/atlx/atl1.c: In function 'atl1_set_link_ksettings':
drivers/net/ethernet/atheros/atlx/atl1.c:3280:6: warning:
 variable 'advertising' set but not used [-Wunused-but-set-variable]

Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 drivers/net/ethernet/atheros/atlx/atl1.c | 21 +--------------------
 1 file changed, 1 insertion(+), 20 deletions(-)

diff --git a/drivers/net/ethernet/atheros/atlx/atl1.c b/drivers/net/ethernet/atheros/atlx/atl1.c
index 72e7fa7..63edc57 100644
--- a/drivers/net/ethernet/atheros/atlx/atl1.c
+++ b/drivers/net/ethernet/atheros/atlx/atl1.c
@@ -3277,7 +3277,6 @@ static int atl1_set_link_ksettings(struct net_device *netdev,
 	u16 phy_data;
 	int ret_val = 0;
 	u16 old_media_type = hw->media_type;
-	u32 advertising;
 
 	if (netif_running(adapter->netdev)) {
 		if (netif_msg_link(adapter))
@@ -3311,25 +3310,7 @@ static int atl1_set_link_ksettings(struct net_device *netdev,
 				hw->media_type = MEDIA_TYPE_10M_HALF;
 		}
 	}
-	switch (hw->media_type) {
-	case MEDIA_TYPE_AUTO_SENSOR:
-		advertising =
-		    ADVERTISED_10baseT_Half |
-		    ADVERTISED_10baseT_Full |
-		    ADVERTISED_100baseT_Half |
-		    ADVERTISED_100baseT_Full |
-		    ADVERTISED_1000baseT_Full |
-		    ADVERTISED_Autoneg | ADVERTISED_TP;
-		break;
-	case MEDIA_TYPE_1000M_FULL:
-		advertising =
-		    ADVERTISED_1000baseT_Full |
-		    ADVERTISED_Autoneg | ADVERTISED_TP;
-		break;
-	default:
-		advertising = 0;
-		break;
-	}
+
 	if (atl1_phy_setup_autoneg_adv(hw)) {
 		ret_val = -EINVAL;
 		if (netif_msg_link(adapter))

^ permalink raw reply related

* [PATCH] net: neterion: remove redundant continue
From: zhong jiang @ 2018-09-20  8:13 UTC (permalink / raw)
  To: davem; +Cc: jdmason, netdev, linux-kernel

The continue will not truely skip any code. hence it is safe to
remove it.

Signed-off-by: zhong jiang <zhongjiang@huawei.com>
---
 drivers/net/ethernet/neterion/s2io.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/neterion/s2io.c b/drivers/net/ethernet/neterion/s2io.c
index f980f10..3c2ac6af 100644
--- a/drivers/net/ethernet/neterion/s2io.c
+++ b/drivers/net/ethernet/neterion/s2io.c
@@ -3679,11 +3679,9 @@ static void restore_xmsi_data(struct s2io_nic *nic)
 		writeq(nic->msix_info[i].data, &bar0->xmsi_data);
 		val64 = (s2BIT(7) | s2BIT(15) | vBIT(msix_index, 26, 6));
 		writeq(val64, &bar0->xmsi_access);
-		if (wait_for_msix_trans(nic, msix_index)) {
+		if (wait_for_msix_trans(nic, msix_index))
 			DBG_PRINT(ERR_DBG, "%s: index: %d failed\n",
 				  __func__, msix_index);
-			continue;
-		}
 	}
 }
 
-- 
1.7.12.4

^ permalink raw reply related


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