Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH v2 2/4] bpf: fix 'struct pt_reg' typo in documentation
From: Quentin Monnet @ 2019-08-21 10:28 UTC (permalink / raw)
  To: Peter Wu, Alexei Starovoitov, Daniel Borkmann; +Cc: netdev, bpf
In-Reply-To: <20190820230900.23445-3-peter@lekensteyn.nl>

2019-08-21 00:08 UTC+0100 ~ Peter Wu <peter@lekensteyn.nl>
> There is no 'struct pt_reg'.
> 
> Signed-off-by: Peter Wu <peter@lekensteyn.nl>

Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>

Thanks for the fix!
Quentin

^ permalink raw reply

* Re: [PATCH net-next v4] sched: Add dualpi2 qdisc
From: Tilmans, Olivier (Nokia - BE/Antwerp) @ 2019-08-21 10:53 UTC (permalink / raw)
  To: Eric Dumazet, Stephen Hemminger, Olga Albisser,
	De Schepper, Koen (Nokia - BE/Antwerp), Bob Briscoe, Henrik Steen,
	Jamal Hadi Salim, Cong Wang, Jiri Pirko, David S. Miller,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <20190821081150.31838-1-olivier.tilmans@nokia-bell-labs.com>

> +static s64 __scale_delta(s64 diff)
> +{
> +	do_div(diff, (1 << (ALPHA_BETA_GRANULARITY + 1)) - 1);
> +	return diff;
> +}
[...]
> +	delta = __scale_delta(((s64)qdelay - q->pi2.target) * q->pi2.alpha);
> +	delta += __scale_delta(((s64)qdelay - qdelay_old) * q->pi2.beta);

I just noticed that ensuring 64b divide compatibility across platforms 
using do_div() introduced this bug, as do_div() works with unsigned operands.

This will be fixed in a later v5 with the following patch:

---
 net/sched/sch_dualpi2.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/net/sched/sch_dualpi2.c b/net/sched/sch_dualpi2.c
index a6452aa82018..c6c851499d35 100644
--- a/net/sched/sch_dualpi2.c
+++ b/net/sched/sch_dualpi2.c
@@ -385,7 +385,7 @@ static struct sk_buff *dualpi2_qdisc_dequeue(struct Qdisc *sch)
 	return skb;
 }
 
-static s64 __scale_delta(s64 diff)
+static s64 __scale_delta(u64 diff)
 {
 	do_div(diff, (1 << (ALPHA_BETA_GRANULARITY + 1)) - 1);
 	return diff;
@@ -406,16 +406,18 @@ static u32 calculate_probability(struct Qdisc *sch)
 	/* Alpha and beta take at most 32b, i.e, the delay difference would
 	 * overflow for queueing delay differences > ~4.2sec.
 	 */
-	delta = __scale_delta(((s64)qdelay - q->pi2.target) * q->pi2.alpha);
-	delta += __scale_delta(((s64)qdelay - qdelay_old) * q->pi2.beta);
-	new_prob = delta + q->pi2.prob;
+	delta = ((s64)qdelay - q->pi2.target) * q->pi2.alpha;
+	delta += ((s64)qdelay - qdelay_old) * q->pi2.beta;
 	/* Prevent overflow */
 	if (delta > 0) {
+		new_prob = __scale_delta(delta) + q->pi2.prob;
 		if (new_prob < q->pi2.prob)
 			new_prob = MAX_PROB;
+	} else {
+		new_prob = q->pi2.prob - __scale_delta(delta * -1);
 		/* Prevent underflow */
-	} else if (new_prob > q->pi2.prob) {
-		new_prob = 0;
+		if (new_prob > q->pi2.prob)
+			new_prob = 0;
 	}
 	/* If we do not drop on overload, ensure we cap the L4S probability to
 	 * 100% to keep window fairness when overflowing.
-- 

Sorry for this.


Best,
Olivier

^ permalink raw reply related

* Re: [PATCH net-next 1/1] Add genphy_c45_config_aneg() function to phy-c45.c
From: Marco Hartmann @ 2019-08-21 10:55 UTC (permalink / raw)
  To: Heiner Kallweit, Christian Herber, andrew@lunn.ch,
	f.fainelli@gmail.com, davem@davemloft.net, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <3b16b8b6-7a9f-0376-ba73-96d23262dd6e@gmail.com>

On 19.08.19 21:51, Heiner Kallweit wrote:
> On 19.08.2019 19:52, Marco Hartmann wrote:
>> and call it from phy_config_aneg().
>>
> Here something went wrong.
> 
>> commit 34786005eca3 ("net: phy: prevent PHYs w/o Clause 22 regs from
>> calling genphy_config_aneg") introduced a check that aborts
>> phy_config_aneg() if the phy is a C45 phy.
>> This causes phy_state_machine() to call phy_error() so that the phy
>> ends up in PHY_HALTED state.
>>
>> Instead of returning -EOPNOTSUPP, call genphy_c45_config_aneg()
>> (analogous to the C22 case) so that the state machine can run
>> correctly.
>>
>> genphy_c45_config_aneg() closely resembles mv3310_config_aneg()
>> in drivers/net/phy/marvell10g.c, excluding vendor specific
>> configurations for 1000BaseT.
>>
>> Fixes: 34786005eca3 ("net: phy: prevent PHYs w/o Clause 22 regs from
>> calling genphy_config_aneg")
>>
> This tag seems to be the wrong one. This change was done before
> genphy_c45_driver was added. Most likely tag should be:
> 22b56e827093 ("net: phy: replace genphy_10g_driver with genphy_c45_driver")
> And because it's a fix applying to previous kernel versions it should
> be annotated "net", not "net-next".
> 
You are correct, I fixed the tag and annotation.

>> Signed-off-by: Marco Hartmann <marco.hartmann@nxp.com>
>> ---
>>   drivers/net/phy/phy-c45.c | 26 ++++++++++++++++++++++++++
>>   drivers/net/phy/phy.c     |  2 +-
>>   include/linux/phy.h       |  1 +
>>   3 files changed, 28 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
>> index b9d4145781ca..fa9062fd9122 100644
>> --- a/drivers/net/phy/phy-c45.c
>> +++ b/drivers/net/phy/phy-c45.c
>> @@ -509,6 +509,32 @@ int genphy_c45_read_status(struct phy_device *phydev)
>>   }
>>   EXPORT_SYMBOL_GPL(genphy_c45_read_status);
>>   
>> +/**
>> + * genphy_c45_config_aneg - restart auto-negotiation or forced setup
>> + * @phydev: target phy_device struct
>> + *
>> + * Description: If auto-negotiation is enabled, we configure the
>> + *   advertising, and then restart auto-negotiation.  If it is not
>> + *   enabled, then we force a configuration.
>> + */
>> +int genphy_c45_config_aneg(struct phy_device *phydev)
>> +{
>> +	int ret;
>> +	bool changed = false;
> 
> Reverse xmas tree please.
> 
ok.

>> [...]
> 
> Overall looks good to me. For a single patch you don't have to provide
> a cover letter.
> 

Thank you for your feedback,
I will provide a v2 of the patch with the above fixes.

Regards,
Marco

^ permalink raw reply

* Re: Regression fix for bpf in v5.3 (was Re: [RFC PATCH] bpf: handle 32-bit zext during constant blinding)
From: Jiong Wang @ 2019-08-21 10:55 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Naveen N. Rao, Alexei Starovoitov, Daniel Borkmann, Jiong Wang,
	bpf, linuxppc-dev, netdev, linux-kernel
In-Reply-To: <87d0gy6cj6.fsf@concordia.ellerman.id.au>


Michael Ellerman writes:

> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> writes:
>> Since BPF constant blinding is performed after the verifier pass, there
>> are certain ALU32 instructions inserted which don't have a corresponding
>> zext instruction inserted after. This is causing a kernel oops on
>> powerpc and can be reproduced by running 'test_cgroup_storage' with
>> bpf_jit_harden=2.
>>
>> Fix this by emitting BPF_ZEXT during constant blinding if
>> prog->aux->verifier_zext is set.
>>
>> Fixes: a4b1d3c1ddf6cb ("bpf: verifier: insert zero extension according to analysis result")
>> Reported-by: Michael Ellerman <mpe@ellerman.id.au>
>> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
>> ---
>> This approach (the location where zext is being introduced below, in 
>> particular) works for powerpc, but I am not entirely sure if this is 
>> sufficient for other architectures as well. This is broken on v5.3-rc4.
>
> Any comment on this?

Have commented on https://marc.info/?l=linux-netdev&m=156637836024743&w=2

The fix looks correct to me on "BPF_LD | BPF_IMM | BPF_DW", but looks
unnecessary on two other places. It would be great if you or Naveen could
confirm it.

Thanks.

Regards,
Jiong

> This is a regression in v5.3, which results in a kernel crash, it would
> be nice to get it fixed before the release please?
>
> cheers
>
>> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
>> index 8191a7db2777..d84146e6fd9e 100644
>> --- a/kernel/bpf/core.c
>> +++ b/kernel/bpf/core.c
>> @@ -890,7 +890,8 @@ int bpf_jit_get_func_addr(const struct bpf_prog *prog,
>>  
>>  static int bpf_jit_blind_insn(const struct bpf_insn *from,
>>  			      const struct bpf_insn *aux,
>> -			      struct bpf_insn *to_buff)
>> +			      struct bpf_insn *to_buff,
>> +			      bool emit_zext)
>>  {
>>  	struct bpf_insn *to = to_buff;
>>  	u32 imm_rnd = get_random_int();
>> @@ -939,6 +940,8 @@ static int bpf_jit_blind_insn(const struct bpf_insn *from,
>>  		*to++ = BPF_ALU32_IMM(BPF_MOV, BPF_REG_AX, imm_rnd ^ from->imm);
>>  		*to++ = BPF_ALU32_IMM(BPF_XOR, BPF_REG_AX, imm_rnd);
>>  		*to++ = BPF_ALU32_REG(from->code, from->dst_reg, BPF_REG_AX);
>> +		if (emit_zext)
>> +			*to++ = BPF_ZEXT_REG(from->dst_reg);
>>  		break;
>>  
>>  	case BPF_ALU64 | BPF_ADD | BPF_K:
>> @@ -992,6 +995,10 @@ static int bpf_jit_blind_insn(const struct bpf_insn *from,
>>  			off -= 2;
>>  		*to++ = BPF_ALU32_IMM(BPF_MOV, BPF_REG_AX, imm_rnd ^ from->imm);
>>  		*to++ = BPF_ALU32_IMM(BPF_XOR, BPF_REG_AX, imm_rnd);
>> +		if (emit_zext) {
>> +			*to++ = BPF_ZEXT_REG(BPF_REG_AX);
>> +			off--;
>> +		}
>>  		*to++ = BPF_JMP32_REG(from->code, from->dst_reg, BPF_REG_AX,
>>  				      off);
>>  		break;
>> @@ -1005,6 +1012,8 @@ static int bpf_jit_blind_insn(const struct bpf_insn *from,
>>  	case 0: /* Part 2 of BPF_LD | BPF_IMM | BPF_DW. */
>>  		*to++ = BPF_ALU32_IMM(BPF_MOV, BPF_REG_AX, imm_rnd ^ aux[0].imm);
>>  		*to++ = BPF_ALU32_IMM(BPF_XOR, BPF_REG_AX, imm_rnd);
>> +		if (emit_zext)
>> +			*to++ = BPF_ZEXT_REG(BPF_REG_AX);
>>  		*to++ = BPF_ALU64_REG(BPF_OR,  aux[0].dst_reg, BPF_REG_AX);
>>  		break;
>>  
>> @@ -1088,7 +1097,8 @@ struct bpf_prog *bpf_jit_blind_constants(struct bpf_prog *prog)
>>  		    insn[1].code == 0)
>>  			memcpy(aux, insn, sizeof(aux));
>>  
>> -		rewritten = bpf_jit_blind_insn(insn, aux, insn_buff);
>> +		rewritten = bpf_jit_blind_insn(insn, aux, insn_buff,
>> +						clone->aux->verifier_zext);
>>  		if (!rewritten)
>>  			continue;
>>  
>> -- 
>> 2.22.0


^ permalink raw reply

* Re: [PATCH spi for-5.4 1/5] spi: Use an abbreviated pointer to ctlr->cur_msg in __spi_pump_messages
From: Mark Brown @ 2019-08-21 11:01 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: Hubert Feurstein, mlichvar, Richard Cochran, Andrew Lunn,
	Florian Fainelli, linux-spi, netdev
In-Reply-To: <CA+h21hpgi7-dJ-QoRBEQorcRyEuhqhKixpFK5fGxOnZxTHi-4g@mail.gmail.com>

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

On Tue, Aug 20, 2019 at 10:36:43PM +0300, Vladimir Oltean wrote:
> On Tue, 20 Aug 2019 at 21:21, Mark Brown <broonie@kernel.org> wrote:

> > On Sun, Aug 18, 2019 at 09:25:56PM +0300, Vladimir Oltean wrote:

> > >       /* Extract head of queue */
> > > -     ctlr->cur_msg =
> > > -             list_first_entry(&ctlr->queue, struct spi_message, queue);
> > > +     mesg = list_first_entry(&ctlr->queue, struct spi_message, queue);
> > > +     ctlr->cur_msg = mesg;

> > Why mesg when the existing code uses msg as an abbreviation here?

> Does it matter? I took from spi_finalize_current_message which also uses mesg.

It's particularly visible when it's on the same line, flags up a
question about if things are the same.  Other things not being great
doesn't preclude making this one better.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* [PATCH v2 net] Add genphy_c45_config_aneg() function to phy-c45.c
From: Marco Hartmann @ 2019-08-21 11:00 UTC (permalink / raw)
  To: andrew@lunn.ch, f.fainelli@gmail.com, hkallweit1@gmail.com,
	davem@davemloft.net, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, Marco Hartmann, Christian Herber

Commit 34786005eca3 ("net: phy: prevent PHYs w/o Clause 22 regs from calling
genphy_config_aneg") introduced a check that aborts phy_config_aneg()
if the phy is a C45 phy.
This causes phy_state_machine() to call phy_error() so that the phy
ends up in PHY_HALTED state.

Instead of returning -EOPNOTSUPP, call genphy_c45_config_aneg()
(analogous to the C22 case) so that the state machine can run
correctly.

genphy_c45_config_aneg() closely resembles mv3310_config_aneg()
in drivers/net/phy/marvell10g.c, excluding vendor specific
configurations for 1000BaseT.

Fixes: 22b56e827093 ("net: phy: replace genphy_10g_driver with genphy_c45_driver")

Signed-off-by: Marco Hartmann <marco.hartmann@nxp.com>
---
Changes in v2:
- corrected commit message
- reordered variables
---
---
 drivers/net/phy/phy-c45.c | 26 ++++++++++++++++++++++++++
 drivers/net/phy/phy.c     |  2 +-
 include/linux/phy.h       |  1 +
 3 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
index 58bb25e4af10..7935593debb1 100644
--- a/drivers/net/phy/phy-c45.c
+++ b/drivers/net/phy/phy-c45.c
@@ -523,6 +523,32 @@ int genphy_c45_read_status(struct phy_device *phydev)
 }
 EXPORT_SYMBOL_GPL(genphy_c45_read_status);
 
+/**
+ * genphy_c45_config_aneg - restart auto-negotiation or forced setup
+ * @phydev: target phy_device struct
+ *
+ * Description: If auto-negotiation is enabled, we configure the
+ *   advertising, and then restart auto-negotiation.  If it is not
+ *   enabled, then we force a configuration.
+ */
+int genphy_c45_config_aneg(struct phy_device *phydev)
+{
+	bool changed = false;
+	int ret;
+
+	if (phydev->autoneg == AUTONEG_DISABLE)
+		return genphy_c45_pma_setup_forced(phydev);
+
+	ret = genphy_c45_an_config_aneg(phydev);
+	if (ret < 0)
+		return ret;
+	if (ret > 0)
+		changed = true;
+
+	return genphy_c45_check_and_restart_aneg(phydev, changed);
+}
+EXPORT_SYMBOL_GPL(genphy_c45_config_aneg);
+
 /* The gen10g_* functions are the old Clause 45 stub */
 
 int gen10g_config_aneg(struct phy_device *phydev)
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index f3adea9ef400..74c4e15ebe52 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -507,7 +507,7 @@ static int phy_config_aneg(struct phy_device *phydev)
 	 * allowed to call genphy_config_aneg()
 	 */
 	if (phydev->is_c45 && !(phydev->c45_ids.devices_in_package & BIT(0)))
-		return -EOPNOTSUPP;
+		return genphy_c45_config_aneg(phydev);
 
 	return genphy_config_aneg(phydev);
 }
diff --git a/include/linux/phy.h b/include/linux/phy.h
index d26779f1fb6b..a7ecbe0e55aa 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1117,6 +1117,7 @@ int genphy_c45_an_disable_aneg(struct phy_device *phydev);
 int genphy_c45_read_mdix(struct phy_device *phydev);
 int genphy_c45_pma_read_abilities(struct phy_device *phydev);
 int genphy_c45_read_status(struct phy_device *phydev);
+int genphy_c45_config_aneg(struct phy_device *phydev);
 
 /* The gen10g_* functions are the old Clause 45 stub */
 int gen10g_config_aneg(struct phy_device *phydev);
-- 
2.7.4


^ permalink raw reply related

* [PATCH] trivial: netns: fix typo in 'struct net.passive' description
From: Mike Rapoport @ 2019-08-21 11:29 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, Mike Rapoport

Replace 'decided' with 'decide' so that comment would be

/* To decide when the network namespace should be freed. */

Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
---
 include/net/net_namespace.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h
index cb668bc2692d..ab40d7afdc54 100644
--- a/include/net/net_namespace.h
+++ b/include/net/net_namespace.h
@@ -52,7 +52,7 @@ struct bpf_prog;
 #define NETDEV_HASHENTRIES (1 << NETDEV_HASHBITS)
 
 struct net {
-	refcount_t		passive;	/* To decided when the network
+	refcount_t		passive;	/* To decide when the network
 						 * namespace should be freed.
 						 */
 	refcount_t		count;		/* To decided when the network
-- 
2.7.4


^ permalink raw reply related

* Re: [PATCH bpf-next 2/2] tools: bpftool: add "bpftool map freeze" subcommand
From: Daniel Borkmann @ 2019-08-21 11:40 UTC (permalink / raw)
  To: Quentin Monnet, Alexei Starovoitov; +Cc: bpf, netdev, oss-drivers
In-Reply-To: <20190821085219.30387-3-quentin.monnet@netronome.com>

On 8/21/19 10:52 AM, Quentin Monnet wrote:
> Add a new subcommand to freeze maps from user space.
> 
> Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
> Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> ---
>   .../bpf/bpftool/Documentation/bpftool-map.rst |  9 +++++
>   tools/bpf/bpftool/bash-completion/bpftool     |  4 +--
>   tools/bpf/bpftool/map.c                       | 34 ++++++++++++++++++-
>   3 files changed, 44 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/bpf/bpftool/Documentation/bpftool-map.rst b/tools/bpf/bpftool/Documentation/bpftool-map.rst
> index 61d1d270eb5e..1c0f7146aab0 100644
> --- a/tools/bpf/bpftool/Documentation/bpftool-map.rst
> +++ b/tools/bpf/bpftool/Documentation/bpftool-map.rst
> @@ -36,6 +36,7 @@ MAP COMMANDS
>   |	**bpftool** **map pop**        *MAP*
>   |	**bpftool** **map enqueue**    *MAP* **value** *VALUE*
>   |	**bpftool** **map dequeue**    *MAP*
> +|	**bpftool** **map freeze**     *MAP*
>   |	**bpftool** **map help**
>   |
>   |	*MAP* := { **id** *MAP_ID* | **pinned** *FILE* }
> @@ -127,6 +128,14 @@ DESCRIPTION
>   	**bpftool map dequeue**  *MAP*
>   		  Dequeue and print **value** from the queue.
>   
> +	**bpftool map freeze**  *MAP*
> +		  Freeze the map as read-only from user space. Entries from a
> +		  frozen map can not longer be updated or deleted with the
> +		  **bpf\ ()** system call. This operation is not reversible,
> +		  and the map remains immutable from user space until its
> +		  destruction. However, read and write permissions for BPF
> +		  programs to the map remain unchanged.

That is not correct, programs that are loaded into the system /after/ the map
has been frozen cannot modify values either, thus read-only from both sides.

Thanks,
Daniel

^ permalink raw reply

* [PATCH v2 net-next] net: fec: add C45 MDIO read/write support
From: Marco Hartmann @ 2019-08-21 11:43 UTC (permalink / raw)
  To: Andy Duan, davem@davemloft.net, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, Christian Herber, Marco Hartmann

IEEE 802.3ae clause 45 defines a modified MDIO protocol that uses a two
staged access model in order to increase the address space.

This patch adds support for C45 MDIO read and write accesses, which are
used whenever the MII_ADDR_C45 flag in the regnum argument is set.
In case it is not set, C22 accesses are used as before.

Signed-off-by: Marco Hartmann <marco.hartmann@nxp.com>
---
Changes in v2:
- use bool variable is_c45
- add missing goto statements
---
---
 drivers/net/ethernet/freescale/fec_main.c | 70 ++++++++++++++++++++++++++++---
 1 file changed, 64 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index c01d3ec3e9af..cb3ce27fb27a 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -208,8 +208,11 @@ MODULE_PARM_DESC(macaddr, "FEC Ethernet MAC address");
 
 /* FEC MII MMFR bits definition */
 #define FEC_MMFR_ST		(1 << 30)
+#define FEC_MMFR_ST_C45		(0)
 #define FEC_MMFR_OP_READ	(2 << 28)
+#define FEC_MMFR_OP_READ_C45	(3 << 28)
 #define FEC_MMFR_OP_WRITE	(1 << 28)
+#define FEC_MMFR_OP_ADDR_WRITE	(0)
 #define FEC_MMFR_PA(v)		((v & 0x1f) << 23)
 #define FEC_MMFR_RA(v)		((v & 0x1f) << 18)
 #define FEC_MMFR_TA		(2 << 16)
@@ -1767,7 +1770,8 @@ static int fec_enet_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
 	struct fec_enet_private *fep = bus->priv;
 	struct device *dev = &fep->pdev->dev;
 	unsigned long time_left;
-	int ret = 0;
+	int ret = 0, frame_start, frame_addr, frame_op;
+	bool is_c45 = !!(regnum & MII_ADDR_C45);
 
 	ret = pm_runtime_get_sync(dev);
 	if (ret < 0)
@@ -1775,9 +1779,37 @@ static int fec_enet_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
 
 	reinit_completion(&fep->mdio_done);
 
+	if (is_c45) {
+		frame_start = FEC_MMFR_ST_C45;
+
+		/* write address */
+		frame_addr = (regnum >> 16);
+		writel(frame_start | FEC_MMFR_OP_ADDR_WRITE |
+		       FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(frame_addr) |
+		       FEC_MMFR_TA | (regnum & 0xFFFF),
+		       fep->hwp + FEC_MII_DATA);
+
+		/* wait for end of transfer */
+		time_left = wait_for_completion_timeout(&fep->mdio_done,
+				usecs_to_jiffies(FEC_MII_TIMEOUT));
+		if (time_left == 0) {
+			netdev_err(fep->netdev, "MDIO address write timeout\n");
+			ret = -ETIMEDOUT;
+			goto out;
+		}
+
+		frame_op = FEC_MMFR_OP_READ_C45;
+
+	} else {
+		/* C22 read */
+		frame_op = FEC_MMFR_OP_READ;
+		frame_start = FEC_MMFR_ST;
+		frame_addr = regnum;
+	}
+
 	/* start a read op */
-	writel(FEC_MMFR_ST | FEC_MMFR_OP_READ |
-		FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(regnum) |
+	writel(frame_start | frame_op |
+		FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(frame_addr) |
 		FEC_MMFR_TA, fep->hwp + FEC_MII_DATA);
 
 	/* wait for end of transfer */
@@ -1804,7 +1836,8 @@ static int fec_enet_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
 	struct fec_enet_private *fep = bus->priv;
 	struct device *dev = &fep->pdev->dev;
 	unsigned long time_left;
-	int ret;
+	int ret, frame_start, frame_addr;
+	bool is_c45 = !!(regnum & MII_ADDR_C45);
 
 	ret = pm_runtime_get_sync(dev);
 	if (ret < 0)
@@ -1814,9 +1847,33 @@ static int fec_enet_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
 
 	reinit_completion(&fep->mdio_done);
 
+	if (is_c45) {
+		frame_start = FEC_MMFR_ST_C45;
+
+		/* write address */
+		frame_addr = (regnum >> 16);
+		writel(frame_start | FEC_MMFR_OP_ADDR_WRITE |
+		       FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(frame_addr) |
+		       FEC_MMFR_TA | (regnum & 0xFFFF),
+		       fep->hwp + FEC_MII_DATA);
+
+		/* wait for end of transfer */
+		time_left = wait_for_completion_timeout(&fep->mdio_done,
+			usecs_to_jiffies(FEC_MII_TIMEOUT));
+		if (time_left == 0) {
+			netdev_err(fep->netdev, "MDIO address write timeout\n");
+			ret = -ETIMEDOUT;
+			goto out;
+		}
+	} else {
+		/* C22 write */
+		frame_start = FEC_MMFR_ST;
+		frame_addr = regnum;
+	}
+
 	/* start a write op */
-	writel(FEC_MMFR_ST | FEC_MMFR_OP_WRITE |
-		FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(regnum) |
+	writel(frame_start | FEC_MMFR_OP_WRITE |
+		FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(frame_addr) |
 		FEC_MMFR_TA | FEC_MMFR_DATA(value),
 		fep->hwp + FEC_MII_DATA);
 
@@ -1828,6 +1885,7 @@ static int fec_enet_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
 		ret  = -ETIMEDOUT;
 	}
 
+out:
 	pm_runtime_mark_last_busy(dev);
 	pm_runtime_put_autosuspend(dev);
 
-- 
2.7.4


^ permalink raw reply related

* Re: [PATCH net-next 1/1] fec: add C45 MDIO read/write support
From: Marco Hartmann @ 2019-08-21 11:44 UTC (permalink / raw)
  To: Andy Duan, davem@davemloft.net, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, Christian Herber
In-Reply-To: <VI1PR0402MB3600576CFF2392A71B1DA99CFFAB0@VI1PR0402MB3600.eurprd04.prod.outlook.com>

On 20.08.19 04:08, Andy Duan wrote:
> From: Marco Hartmann Sent: Tuesday, August 20, 2019 1:11 AM
>> IEEE 802.3ae clause 45 defines a modified MDIO protocol that uses a two
>> staged access model in order to increase the address space.
>>
>> This patch adds support for C45 MDIO read and write accesses, which are
>> used whenever the MII_ADDR_C45 flag in the regnum argument is set.
>> In case it is not set, C22 accesses are used as before.
>>
>> Co-developed-by: Christian Herber <christian.herber@nxp.com>
>> Signed-off-by: Christian Herber <christian.herber@nxp.com>
>> Signed-off-by: Marco Hartmann <marco.hartmann@nxp.com>
>> ---
>>   drivers/net/ethernet/freescale/fec_main.c | 65
>> ++++++++++++++++++++++++++++---
>>   1 file changed, 59 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/freescale/fec_main.c
>> b/drivers/net/ethernet/freescale/fec_main.c
>> index c01d3ec3e9af..73f8f9a149a1 100644
>> --- a/drivers/net/ethernet/freescale/fec_main.c
>> +++ b/drivers/net/ethernet/freescale/fec_main.c
>> @@ -208,8 +208,11 @@ MODULE_PARM_DESC(macaddr, "FEC Ethernet
>> MAC address");
>>
>>   /* FEC MII MMFR bits definition */
>>   #define FEC_MMFR_ST		(1 << 30)
>> +#define FEC_MMFR_ST_C45		(0)
>>   #define FEC_MMFR_OP_READ	(2 << 28)
>> +#define FEC_MMFR_OP_READ_C45	(3 << 28)
>>   #define FEC_MMFR_OP_WRITE	(1 << 28)
>> +#define FEC_MMFR_OP_ADDR_WRITE	(0)
>>   #define FEC_MMFR_PA(v)		((v & 0x1f) << 23)
>>   #define FEC_MMFR_RA(v)		((v & 0x1f) << 18)
>>   #define FEC_MMFR_TA		(2 << 16)
>> @@ -1767,7 +1770,7 @@ static int fec_enet_mdio_read(struct mii_bus *bus,
>> int mii_id, int regnum)
>>   	struct fec_enet_private *fep = bus->priv;
>>   	struct device *dev = &fep->pdev->dev;
>>   	unsigned long time_left;
>> -	int ret = 0;
>> +	int ret = 0, frame_start, frame_addr, frame_op;
> 
> Add bool variable:
> 
> bool is_c45 = !!(regnum & MII_ADDR_C45);
>>
>>   	ret = pm_runtime_get_sync(dev);
>>   	if (ret < 0)
>> @@ -1775,9 +1778,36 @@ static int fec_enet_mdio_read(struct mii_bus
>> *bus, int mii_id, int regnum)
>>
>>   	reinit_completion(&fep->mdio_done);
>>
>> +	if (MII_ADDR_C45 & regnum) {
> if (is_c45)
> 
>> +		frame_start = FEC_MMFR_ST_C45;
>> +
>> +		/* write address */
>> +		frame_addr = (regnum >> 16);
>> +		writel(frame_start | FEC_MMFR_OP_ADDR_WRITE |
>> +		       FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(frame_addr) |
>> +		       FEC_MMFR_TA | (regnum & 0xFFFF),
>> +		       fep->hwp + FEC_MII_DATA);
>> +
>> +		/* wait for end of transfer */
>> +		time_left = wait_for_completion_timeout(&fep->mdio_done,
>> +				usecs_to_jiffies(FEC_MII_TIMEOUT));
>> +		if (time_left == 0) {
>> +			netdev_err(fep->netdev, "MDIO address write timeout\n");
>> +			ret  = -ETIMEDOUT;
> 
> Should be:
> goto out;
>> +		}
>> +
>> +		frame_op = FEC_MMFR_OP_READ_C45;
>> +
>> +	} else {
>> +		/* C22 read */
>> +		frame_op = FEC_MMFR_OP_READ;
>> +		frame_start = FEC_MMFR_ST;
>> +		frame_addr = regnum;
>> +	}
>> +
>>   	/* start a read op */
>> -	writel(FEC_MMFR_ST | FEC_MMFR_OP_READ |
>> -		FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(regnum) |
>> +	writel(frame_start | frame_op |
>> +		FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(frame_addr) |
>>   		FEC_MMFR_TA, fep->hwp + FEC_MII_DATA);
>>
>>   	/* wait for end of transfer */
>> @@ -1804,7 +1834,7 @@ static int fec_enet_mdio_write(struct mii_bus *bus,
>> int mii_id, int regnum,
>>   	struct fec_enet_private *fep = bus->priv;
>>   	struct device *dev = &fep->pdev->dev;
>>   	unsigned long time_left;
>> -	int ret;
>> +	int ret, frame_start, frame_addr;
>>
>>   	ret = pm_runtime_get_sync(dev);
>>   	if (ret < 0)
>> @@ -1814,9 +1844,32 @@ static int fec_enet_mdio_write(struct mii_bus
>> *bus, int mii_id, int regnum,
> 
> bool is_c45 = !!(regnum & MII_ADDR_C45);
>>
>>   	reinit_completion(&fep->mdio_done);
>>
>> +	if (MII_ADDR_C45 & regnum) {
> 
> if (!is_c45) {
>> +		frame_start = FEC_MMFR_ST_C45;
>> +
>> +		/* write address */
>> +		frame_addr = (regnum >> 16);
>> +		writel(frame_start | FEC_MMFR_OP_ADDR_WRITE |
>> +		       FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(frame_addr) |
>> +		       FEC_MMFR_TA | (regnum & 0xFFFF),
>> +		       fep->hwp + FEC_MII_DATA);
>> +
>> +		/* wait for end of transfer */
>> +		time_left = wait_for_completion_timeout(&fep->mdio_done,
>> +			usecs_to_jiffies(FEC_MII_TIMEOUT));
>> +		if (time_left == 0) {
>> +			netdev_err(fep->netdev, "MDIO address write timeout\n");
>> +			ret  = -ETIMEDOUT;
> Like mdio read, it should be:
> goto out;
>> +		}
>> +	} else {
>> +		/* C22 write */
>> +		frame_start = FEC_MMFR_ST;
>> +		frame_addr = regnum;
>> +	}
>> +
>>   	/* start a write op */
>> -	writel(FEC_MMFR_ST | FEC_MMFR_OP_WRITE |
>> -		FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(regnum) |
>> +	writel(frame_start | FEC_MMFR_OP_WRITE |
>> +		FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(frame_addr) |
>>   		FEC_MMFR_TA | FEC_MMFR_DATA(value),
>>   		fep->hwp + FEC_MII_DATA);
>>
>> --
>> 2.7.4
> 

Thank you for your feedback,
the fixes are included in v2 of the patch.

Regards,
Marco

^ permalink raw reply

* Re: [PATCH net-next v2 6/9] net: macsec: hardware offloading infrastructure
From: Igor Russkikh @ 2019-08-21 12:01 UTC (permalink / raw)
  To: Antoine Tenart, Sabrina Dubroca
  Cc: Andrew Lunn, davem@davemloft.net, f.fainelli@gmail.com,
	hkallweit1@gmail.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, thomas.petazzoni@bootlin.com,
	alexandre.belloni@bootlin.com, allan.nielsen@microchip.com,
	camelia.groza@nxp.com, Simon Edelhaus, Pavel Belous
In-Reply-To: <20190821100106.GA3006@kwain>


> Right. I did not see how *not* to strip the sectag in the h/w back then,
> I'll have another look because that would improve things a lot.
> 
> @all: do other MACsec offloading hardware allow not to stip the sectag?

I've just checked this, and see an action option in our HW classifier to keep
macsec header with optional error information added. But we've never
experimented configuring this honestly, I don't think we should rely in general
design that such a feature is widely available in HW.

Regards,
  Igor

^ permalink raw reply

* Re: [PATCH bpf-next v2 2/4] selftests/bpf: test_progs: remove global fail/success counts
From: Daniel Borkmann @ 2019-08-21 12:17 UTC (permalink / raw)
  To: Stanislav Fomichev, netdev, bpf; +Cc: davem, ast, Andrii Nakryiko
In-Reply-To: <20190819191752.241637-3-sdf@google.com>

On 8/19/19 9:17 PM, Stanislav Fomichev wrote:
> Now that we have a global per-test/per-environment state, there
> is no longer need to have global fail/success counters (and there
> is no need to save/get the diff before/after the test).

Thanks for the improvements, just a small comment below, otherwise LGTM.

> Introduce QCHECK macro (suggested by Andrii) and covert existing tests
> to it. QCHECK uses new test__fail() to record the failure.
> 
> Cc: Andrii Nakryiko <andriin@fb.com>
> Signed-off-by: Stanislav Fomichev <sdf@google.com>
[...]
> @@ -96,17 +93,25 @@ extern struct ipv6_packet pkt_v6;
>   #define _CHECK(condition, tag, duration, format...) ({			\
>   	int __ret = !!(condition);					\
>   	if (__ret) {							\
> -		error_cnt++;						\
> +		test__fail();						\
>   		printf("%s:FAIL:%s ", __func__, tag);			\
>   		printf(format);						\
>   	} else {							\
> -		pass_cnt++;						\
>   		printf("%s:PASS:%s %d nsec\n",				\
>   		       __func__, tag, duration);			\
>   	}								\
>   	__ret;								\
>   })
>   
> +#define QCHECK(condition) ({						\
> +	int __ret = !!(condition);					\
> +	if (__ret) {							\
> +		test__fail();						\
> +		printf("%s:FAIL:%d ", __func__, __LINE__);		\
> +	}								\
> +	__ret;								\
> +})

I know it's just a tiny nit but the name QCHECK() really doesn't tell me anything
if I don't see its definition. Even just a CHECK_FAIL() might be 'better' and
more aligned with the CHECK() and CHECK_ATTR() we have, at least I don't think
many would automatically derive 'quiet' from the Q prefix [0].

   [0] https://lore.kernel.org/bpf/CAEf4BzbUGiUZBWkTWe2=LfhkXYhQGndN9gR6VTZwfV3eytstUw@mail.gmail.com/

>   #define CHECK(condition, tag, format...) \
>   	_CHECK(condition, tag, duration, format)
>   #define CHECK_ATTR(condition, tag, format...) \
> 


^ permalink raw reply

* [PATCH bpf] flow_dissector: Fix potential use-after-free on BPF_PROG_DETACH
From: Jakub Sitnicki @ 2019-08-21 12:17 UTC (permalink / raw)
  To: bpf; +Cc: netdev, kernel-team, Petar Penkov, Willem de Bruijn, Lorenz Bauer

Call to bpf_prog_put(), with help of call_rcu(), queues an RCU-callback to
free the program once a grace period has elapsed. The callback can run
together with new RCU readers that started after the last grace period.
New RCU readers can potentially see the "old" to-be-freed or already-freed
pointer to the program object before the RCU update-side NULLs it.

Reorder the operations so that the RCU update-side resets the protected
pointer before the end of the grace period after which the program will be
freed.

Fixes: d58e468b1112 ("flow_dissector: implements flow dissector BPF hook")
Reported-by: Lorenz Bauer <lmb@cloudflare.com>
Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com>
---
 net/core/flow_dissector.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 3e6fedb57bc1..2470b4b404e6 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -142,8 +142,8 @@ int skb_flow_dissector_bpf_prog_detach(const union bpf_attr *attr)
 		mutex_unlock(&flow_dissector_mutex);
 		return -ENOENT;
 	}
-	bpf_prog_put(attached);
 	RCU_INIT_POINTER(net->flow_dissector_prog, NULL);
+	bpf_prog_put(attached);
 	mutex_unlock(&flow_dissector_mutex);
 	return 0;
 }
-- 
2.20.1


^ permalink raw reply related

* [PATCH net-next] amd-xgbe: use devm_platform_ioremap_resource() to simplify code
From: YueHaibing @ 2019-08-21 12:32 UTC (permalink / raw)
  To: davem, thomas.lendacky; +Cc: linux-kernel, netdev, YueHaibing

Use devm_platform_ioremap_resource() to simplify the code a bit.
This is detected by coccinelle.

Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 drivers/net/ethernet/amd/xgbe/xgbe-platform.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-platform.c b/drivers/net/ethernet/amd/xgbe/xgbe-platform.c
index dce9e59..4ebd241 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-platform.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-platform.c
@@ -301,7 +301,6 @@ static int xgbe_platform_probe(struct platform_device *pdev)
 	struct xgbe_prv_data *pdata;
 	struct device *dev = &pdev->dev;
 	struct platform_device *phy_pdev;
-	struct resource *res;
 	const char *phy_mode;
 	unsigned int phy_memnum, phy_irqnum;
 	unsigned int dma_irqnum, dma_irqend;
@@ -353,8 +352,7 @@ static int xgbe_platform_probe(struct platform_device *pdev)
 	}
 
 	/* Obtain the mmio areas for the device */
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	pdata->xgmac_regs = devm_ioremap_resource(dev, res);
+	pdata->xgmac_regs = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(pdata->xgmac_regs)) {
 		dev_err(dev, "xgmac ioremap failed\n");
 		ret = PTR_ERR(pdata->xgmac_regs);
@@ -363,8 +361,7 @@ static int xgbe_platform_probe(struct platform_device *pdev)
 	if (netif_msg_probe(pdata))
 		dev_dbg(dev, "xgmac_regs = %p\n", pdata->xgmac_regs);
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
-	pdata->xpcs_regs = devm_ioremap_resource(dev, res);
+	pdata->xpcs_regs = devm_platform_ioremap_resource(pdev, 1);
 	if (IS_ERR(pdata->xpcs_regs)) {
 		dev_err(dev, "xpcs ioremap failed\n");
 		ret = PTR_ERR(pdata->xpcs_regs);
@@ -373,8 +370,8 @@ static int xgbe_platform_probe(struct platform_device *pdev)
 	if (netif_msg_probe(pdata))
 		dev_dbg(dev, "xpcs_regs  = %p\n", pdata->xpcs_regs);
 
-	res = platform_get_resource(phy_pdev, IORESOURCE_MEM, phy_memnum++);
-	pdata->rxtx_regs = devm_ioremap_resource(dev, res);
+	pdata->rxtx_regs = devm_platform_ioremap_resource(phy_pdev,
+							  phy_memnum++);
 	if (IS_ERR(pdata->rxtx_regs)) {
 		dev_err(dev, "rxtx ioremap failed\n");
 		ret = PTR_ERR(pdata->rxtx_regs);
@@ -383,8 +380,8 @@ static int xgbe_platform_probe(struct platform_device *pdev)
 	if (netif_msg_probe(pdata))
 		dev_dbg(dev, "rxtx_regs  = %p\n", pdata->rxtx_regs);
 
-	res = platform_get_resource(phy_pdev, IORESOURCE_MEM, phy_memnum++);
-	pdata->sir0_regs = devm_ioremap_resource(dev, res);
+	pdata->sir0_regs = devm_platform_ioremap_resource(phy_pdev,
+							  phy_memnum++);
 	if (IS_ERR(pdata->sir0_regs)) {
 		dev_err(dev, "sir0 ioremap failed\n");
 		ret = PTR_ERR(pdata->sir0_regs);
@@ -393,8 +390,8 @@ static int xgbe_platform_probe(struct platform_device *pdev)
 	if (netif_msg_probe(pdata))
 		dev_dbg(dev, "sir0_regs  = %p\n", pdata->sir0_regs);
 
-	res = platform_get_resource(phy_pdev, IORESOURCE_MEM, phy_memnum++);
-	pdata->sir1_regs = devm_ioremap_resource(dev, res);
+	pdata->sir1_regs = devm_platform_ioremap_resource(phy_pdev,
+							  phy_memnum++);
 	if (IS_ERR(pdata->sir1_regs)) {
 		dev_err(dev, "sir1 ioremap failed\n");
 		ret = PTR_ERR(pdata->sir1_regs);
-- 
2.7.4



^ permalink raw reply related

* Re: [PATCH bpf-next v2 0/3] xdpsock: allow mmap2 usage for 32bits
From: Daniel Borkmann @ 2019-08-21 12:35 UTC (permalink / raw)
  To: Ivan Khoronzhuk, magnus.karlsson, bjorn.topel
  Cc: davem, hawk, john.fastabend, jakub.kicinski, netdev, bpf,
	xdp-newbies, linux-kernel, jlemon, yhs, andrii.nakryiko
In-Reply-To: <20190815121356.8848-1-ivan.khoronzhuk@linaro.org>

On 8/15/19 2:13 PM, Ivan Khoronzhuk wrote:
> This patchset contains several improvements for af_xdp socket umem
> mappings for 32bit systems. Also, there is one more patch outside of
> this series that on linux-next tree and related to mmap2 af_xdp umem
> offsets: "mm: mmap: increase sockets maximum memory size pgoff for 32bits"
> https://lkml.org/lkml/2019/8/12/549
> 
> Based on bpf-next/master
> 
> Prev: https://lkml.org/lkml/2019/8/13/437
> 
> v2..v1:
> 	- replaced "libbpf: add asm/unistd.h to xsk to get __NR_mmap2" on
> 	 "libbpf: use LFS (_FILE_OFFSET_BITS) instead of direct mmap2
> 	 syscall"
> 	- use vmap along with page_address to avoid overkill
> 	- define mmap syscall trace5 for mmap if defined
> 
> Ivan Khoronzhuk (3):
>    libbpf: use LFS (_FILE_OFFSET_BITS) instead of direct mmap2 syscall
>    xdp: xdp_umem: replace kmap on vmap for umem map
>    samples: bpf: syscal_nrs: use mmap2 if defined
> 
>   net/xdp/xdp_umem.c         | 36 +++++++++++++++++++++++-----
>   samples/bpf/syscall_nrs.c  |  6 +++++
>   samples/bpf/tracex5_kern.c | 13 ++++++++++
>   tools/lib/bpf/Makefile     |  1 +
>   tools/lib/bpf/xsk.c        | 49 +++++++++++---------------------------
>   5 files changed, 64 insertions(+), 41 deletions(-)
> 

Applied, and fixed up typo in last one's subject, thanks!

^ permalink raw reply

* Re: [PATCH v5 11/17] net: sgi: ioc3-eth: no need to stop queue set_multicast_list
From: Thomas Bogendoerfer @ 2019-08-21 12:40 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Jonathan Corbet, Ralf Baechle, Paul Burton, James Hogan,
	Dmitry Torokhov, Lee Jones, David S. Miller, Srinivas Kandagatla,
	Alessandro Zummo, Alexandre Belloni, Greg Kroah-Hartman,
	Jiri Slaby, Evgeniy Polyakov, linux-doc, linux-kernel, linux-mips,
	linux-input, netdev, linux-rtc, linux-serial
In-Reply-To: <20190819170440.37ff18d4@cakuba.netronome.com>

On Mon, 19 Aug 2019 17:04:53 -0700
Jakub Kicinski <jakub.kicinski@netronome.com> wrote:

> On Mon, 19 Aug 2019 18:31:34 +0200, Thomas Bogendoerfer wrote:
> > netif_stop_queue()/netif_wake_qeue() aren't needed for changing
> > multicast filters. Use spinlocks instead for proper protection
> > of private struct.
> > 
>
> I thought it may protect ip->emcr, but that one is accessed with no
> locking from the ioc3_timer() -> ioc3_setup_duplex() path..

it should protect ip->emcr ... I'll add spin_lock/unlock to setup_duplex and
respin the patch.

Thomas.

-- 
SUSE Linux GmbH
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)

^ permalink raw reply

* [PATCH net-next] net: ethernet: ti: use devm_platform_ioremap_resource() to simplify code
From: YueHaibing @ 2019-08-21 12:48 UTC (permalink / raw)
  To: davem, grygorii.strashko, ivan.khoronzhuk, andrew, ynezz
  Cc: linux-kernel, netdev, linux-omap, YueHaibing

Use devm_platform_ioremap_resource() to simplify the code a bit.
This is detected by coccinelle.

Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 drivers/net/ethernet/ti/cpsw.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 32a8974..5401095 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -2764,7 +2764,7 @@ static int cpsw_probe(struct platform_device *pdev)
 	struct net_device		*ndev;
 	struct cpsw_priv		*priv;
 	void __iomem			*ss_regs;
-	struct resource			*res, *ss_res;
+	struct resource			*ss_res;
 	struct gpio_descs		*mode;
 	const struct soc_device_attribute *soc;
 	struct cpsw_common		*cpsw;
@@ -2798,8 +2798,7 @@ static int cpsw_probe(struct platform_device *pdev)
 		return PTR_ERR(ss_regs);
 	cpsw->regs = ss_regs;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
-	cpsw->wr_regs = devm_ioremap_resource(dev, res);
+	cpsw->wr_regs = devm_platform_ioremap_resource(pdev, 1);
 	if (IS_ERR(cpsw->wr_regs))
 		return PTR_ERR(cpsw->wr_regs);
 
-- 
2.7.4



^ permalink raw reply related

* [PATCH net-next] via-rhine: use devm_platform_ioremap_resource() to simplify code
From: YueHaibing @ 2019-08-21 12:50 UTC (permalink / raw)
  To: davem, will; +Cc: linux-kernel, netdev, YueHaibing

Use devm_platform_ioremap_resource() to simplify the code a bit.
This is detected by coccinelle.

Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 drivers/net/ethernet/via/via-rhine.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/via/via-rhine.c b/drivers/net/ethernet/via/via-rhine.c
index ab55416..ed12dbd 100644
--- a/drivers/net/ethernet/via/via-rhine.c
+++ b/drivers/net/ethernet/via/via-rhine.c
@@ -1127,15 +1127,13 @@ static int rhine_init_one_platform(struct platform_device *pdev)
 	const struct of_device_id *match;
 	const u32 *quirks;
 	int irq;
-	struct resource *res;
 	void __iomem *ioaddr;
 
 	match = of_match_device(rhine_of_tbl, &pdev->dev);
 	if (!match)
 		return -EINVAL;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	ioaddr = devm_ioremap_resource(&pdev->dev, res);
+	ioaddr = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(ioaddr))
 		return PTR_ERR(ioaddr);
 
-- 
2.7.4



^ permalink raw reply related

* [PATCH net-next] net: socionext: use devm_platform_ioremap_resource() to simplify code
From: YueHaibing @ 2019-08-21 12:53 UTC (permalink / raw)
  To: davem, hayashi.kunihiko; +Cc: linux-kernel, netdev, YueHaibing

Use devm_platform_ioremap_resource() to simplify the code a bit.
This is detected by coccinelle.

Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 drivers/net/ethernet/socionext/sni_ave.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/socionext/sni_ave.c b/drivers/net/ethernet/socionext/sni_ave.c
index 87ab0b5..10d0c3e 100644
--- a/drivers/net/ethernet/socionext/sni_ave.c
+++ b/drivers/net/ethernet/socionext/sni_ave.c
@@ -1553,7 +1553,6 @@ static int ave_probe(struct platform_device *pdev)
 	struct ave_private *priv;
 	struct net_device *ndev;
 	struct device_node *np;
-	struct resource	*res;
 	const void *mac_addr;
 	void __iomem *base;
 	const char *name;
@@ -1576,8 +1575,7 @@ static int ave_probe(struct platform_device *pdev)
 	if (irq < 0)
 		return irq;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	base = devm_ioremap_resource(dev, res);
+	base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(base))
 		return PTR_ERR(base);
 
-- 
2.7.4



^ permalink raw reply related

* [PATCH net-next] net: ks8851-ml: use devm_platform_ioremap_resource() to simplify code
From: YueHaibing @ 2019-08-21 12:58 UTC (permalink / raw)
  To: davem, ynezz, allison, lukas; +Cc: linux-kernel, netdev, YueHaibing

Use devm_platform_ioremap_resource() to simplify the code a bit.
This is detected by coccinelle.

Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 drivers/net/ethernet/micrel/ks8851_mll.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/micrel/ks8851_mll.c b/drivers/net/ethernet/micrel/ks8851_mll.c
index e52b015..a41a90c 100644
--- a/drivers/net/ethernet/micrel/ks8851_mll.c
+++ b/drivers/net/ethernet/micrel/ks8851_mll.c
@@ -1225,7 +1225,6 @@ MODULE_DEVICE_TABLE(of, ks8851_ml_dt_ids);
 static int ks8851_probe(struct platform_device *pdev)
 {
 	int err;
-	struct resource *io_d, *io_c;
 	struct net_device *netdev;
 	struct ks_net *ks;
 	u16 id, data;
@@ -1240,15 +1239,13 @@ static int ks8851_probe(struct platform_device *pdev)
 	ks = netdev_priv(netdev);
 	ks->netdev = netdev;
 
-	io_d = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	ks->hw_addr = devm_ioremap_resource(&pdev->dev, io_d);
+	ks->hw_addr = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(ks->hw_addr)) {
 		err = PTR_ERR(ks->hw_addr);
 		goto err_free;
 	}
 
-	io_c = platform_get_resource(pdev, IORESOURCE_MEM, 1);
-	ks->hw_addr_cmd = devm_ioremap_resource(&pdev->dev, io_c);
+	ks->hw_addr_cmd = devm_platform_ioremap_resource(pdev, 1);
 	if (IS_ERR(ks->hw_addr_cmd)) {
 		err = PTR_ERR(ks->hw_addr_cmd);
 		goto err_free;
-- 
2.7.4



^ permalink raw reply related

* Re: [PATCH bpf-next 2/2] tools: bpftool: add "bpftool map freeze" subcommand
From: Quentin Monnet @ 2019-08-21 12:58 UTC (permalink / raw)
  To: Daniel Borkmann, Alexei Starovoitov; +Cc: bpf, netdev, oss-drivers
In-Reply-To: <b44cf34c-b6d5-a3f5-f386-e70791e47229@iogearbox.net>

2019-08-21 13:40 UTC+0200 ~ Daniel Borkmann <daniel@iogearbox.net>
> On 8/21/19 10:52 AM, Quentin Monnet wrote:
>> Add a new subcommand to freeze maps from user space.
>>
>> Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
>> Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
>> ---
>>   .../bpf/bpftool/Documentation/bpftool-map.rst |  9 +++++
>>   tools/bpf/bpftool/bash-completion/bpftool     |  4 +--
>>   tools/bpf/bpftool/map.c                       | 34 ++++++++++++++++++-
>>   3 files changed, 44 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/bpf/bpftool/Documentation/bpftool-map.rst
>> b/tools/bpf/bpftool/Documentation/bpftool-map.rst
>> index 61d1d270eb5e..1c0f7146aab0 100644
>> --- a/tools/bpf/bpftool/Documentation/bpftool-map.rst
>> +++ b/tools/bpf/bpftool/Documentation/bpftool-map.rst
>> @@ -36,6 +36,7 @@ MAP COMMANDS
>>   |    **bpftool** **map pop**        *MAP*
>>   |    **bpftool** **map enqueue**    *MAP* **value** *VALUE*
>>   |    **bpftool** **map dequeue**    *MAP*
>> +|    **bpftool** **map freeze**     *MAP*
>>   |    **bpftool** **map help**
>>   |
>>   |    *MAP* := { **id** *MAP_ID* | **pinned** *FILE* }
>> @@ -127,6 +128,14 @@ DESCRIPTION
>>       **bpftool map dequeue**  *MAP*
>>             Dequeue and print **value** from the queue.
>>   +    **bpftool map freeze**  *MAP*
>> +          Freeze the map as read-only from user space. Entries from a
>> +          frozen map can not longer be updated or deleted with the
>> +          **bpf\ ()** system call. This operation is not reversible,
>> +          and the map remains immutable from user space until its
>> +          destruction. However, read and write permissions for BPF
>> +          programs to the map remain unchanged.
> 
> That is not correct, programs that are loaded into the system /after/
> the map
> has been frozen cannot modify values either, thus read-only from both
> sides.
> 
> Thanks,
> Daniel

Hi Daniel,

Are you entirely sure about it? I could not find the relevant
restriction in the code, the checks seem to be on map flags
(BPF_F_RDONLY) which do not seem to be modified by the "frozen" status
in map_freeze()? And tests I ran on my side seem to indicate the map can
still be updated by new programs. Did I miss something?

Tested on 5.3.0-rc1:

1. Create hash map
2. Load BPF program foo, using map
3. Test-run program foo - map is updated
4. Freeze map - update effectively becomes impossible from user space
5. Load BPF program bar, using same map
6. Test-run program bar - map is still updated

Best regards,
Quentin

^ permalink raw reply

* [PATCH net-next] cirrus: cs89x0: use devm_platform_ioremap_resource() to simplify code
From: YueHaibing @ 2019-08-21 13:02 UTC (permalink / raw)
  To: davem; +Cc: linux-kernel, netdev, YueHaibing

Use devm_platform_ioremap_resource() to simplify the code a bit.
This is detected by coccinelle.

Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 drivers/net/ethernet/cirrus/cs89x0.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/cirrus/cs89x0.c b/drivers/net/ethernet/cirrus/cs89x0.c
index b3e7faf..2d30972 100644
--- a/drivers/net/ethernet/cirrus/cs89x0.c
+++ b/drivers/net/ethernet/cirrus/cs89x0.c
@@ -1845,7 +1845,6 @@ static int __init cs89x0_platform_probe(struct platform_device *pdev)
 {
 	struct net_device *dev = alloc_etherdev(sizeof(struct net_local));
 	struct net_local *lp;
-	struct resource *mem_res;
 	void __iomem *virt_addr;
 	int err;
 
@@ -1861,8 +1860,7 @@ static int __init cs89x0_platform_probe(struct platform_device *pdev)
 		goto free;
 	}
 
-	mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	virt_addr = devm_ioremap_resource(&pdev->dev, mem_res);
+	virt_addr = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(virt_addr)) {
 		err = PTR_ERR(virt_addr);
 		goto free;
-- 
2.7.4



^ permalink raw reply related

* [PATCH net-next] net: sxgbe: use devm_platform_ioremap_resource() to simplify code
From: YueHaibing @ 2019-08-21 12:59 UTC (permalink / raw)
  To: davem, bh74.an, ks.giri, vipul.pandya; +Cc: linux-kernel, netdev, YueHaibing

Use devm_platform_ioremap_resource() to simplify the code a bit.
This is detected by coccinelle.

Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 drivers/net/ethernet/samsung/sxgbe/sxgbe_platform.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/samsung/sxgbe/sxgbe_platform.c b/drivers/net/ethernet/samsung/sxgbe/sxgbe_platform.c
index d2c4811..2412c87 100644
--- a/drivers/net/ethernet/samsung/sxgbe/sxgbe_platform.c
+++ b/drivers/net/ethernet/samsung/sxgbe/sxgbe_platform.c
@@ -78,7 +78,6 @@ static int sxgbe_platform_probe(struct platform_device *pdev)
 {
 	int ret;
 	int i, chan;
-	struct resource *res;
 	struct device *dev = &pdev->dev;
 	void __iomem *addr;
 	struct sxgbe_priv_data *priv = NULL;
@@ -88,8 +87,7 @@ static int sxgbe_platform_probe(struct platform_device *pdev)
 	struct device_node *node = dev->of_node;
 
 	/* Get memory resource */
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	addr = devm_ioremap_resource(dev, res);
+	addr = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(addr))
 		return PTR_ERR(addr);
 
-- 
2.7.4



^ permalink raw reply related

* [PATCH net-next] ezchip: nps_enet: use devm_platform_ioremap_resource() to simplify code
From: YueHaibing @ 2019-08-21 13:05 UTC (permalink / raw)
  To: davem, ynezz, tglx, gregkh; +Cc: linux-kernel, netdev, YueHaibing

Use devm_platform_ioremap_resource() to simplify the code a bit.
This is detected by coccinelle.

Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 drivers/net/ethernet/ezchip/nps_enet.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/ezchip/nps_enet.c b/drivers/net/ethernet/ezchip/nps_enet.c
index 027225e..815fb62 100644
--- a/drivers/net/ethernet/ezchip/nps_enet.c
+++ b/drivers/net/ethernet/ezchip/nps_enet.c
@@ -576,7 +576,6 @@ static s32 nps_enet_probe(struct platform_device *pdev)
 	struct nps_enet_priv *priv;
 	s32 err = 0;
 	const char *mac_addr;
-	struct resource *res_regs;
 
 	if (!dev->of_node)
 		return -ENODEV;
@@ -595,8 +594,7 @@ static s32 nps_enet_probe(struct platform_device *pdev)
 	/* FIXME :: no multicast support yet */
 	ndev->flags &= ~IFF_MULTICAST;
 
-	res_regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	priv->regs_base = devm_ioremap_resource(dev, res_regs);
+	priv->regs_base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(priv->regs_base)) {
 		err = PTR_ERR(priv->regs_base);
 		goto out_netdev;
-- 
2.7.4



^ permalink raw reply related

* Re: [PATCH bpf-next 2/2] tools: bpftool: add "bpftool map freeze" subcommand
From: Daniel Borkmann @ 2019-08-21 13:08 UTC (permalink / raw)
  To: Quentin Monnet, Alexei Starovoitov; +Cc: bpf, netdev, oss-drivers
In-Reply-To: <2b6d7326-fc74-288b-fa52-b79752222123@netronome.com>

On 8/21/19 2:58 PM, Quentin Monnet wrote:
> 2019-08-21 13:40 UTC+0200 ~ Daniel Borkmann <daniel@iogearbox.net>
>> On 8/21/19 10:52 AM, Quentin Monnet wrote:
>>> Add a new subcommand to freeze maps from user space.
>>>
>>> Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
>>> Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
>>> ---
>>>    .../bpf/bpftool/Documentation/bpftool-map.rst |  9 +++++
>>>    tools/bpf/bpftool/bash-completion/bpftool     |  4 +--
>>>    tools/bpf/bpftool/map.c                       | 34 ++++++++++++++++++-
>>>    3 files changed, 44 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/tools/bpf/bpftool/Documentation/bpftool-map.rst
>>> b/tools/bpf/bpftool/Documentation/bpftool-map.rst
>>> index 61d1d270eb5e..1c0f7146aab0 100644
>>> --- a/tools/bpf/bpftool/Documentation/bpftool-map.rst
>>> +++ b/tools/bpf/bpftool/Documentation/bpftool-map.rst
>>> @@ -36,6 +36,7 @@ MAP COMMANDS
>>>    |    **bpftool** **map pop**        *MAP*
>>>    |    **bpftool** **map enqueue**    *MAP* **value** *VALUE*
>>>    |    **bpftool** **map dequeue**    *MAP*
>>> +|    **bpftool** **map freeze**     *MAP*
>>>    |    **bpftool** **map help**
>>>    |
>>>    |    *MAP* := { **id** *MAP_ID* | **pinned** *FILE* }
>>> @@ -127,6 +128,14 @@ DESCRIPTION
>>>        **bpftool map dequeue**  *MAP*
>>>              Dequeue and print **value** from the queue.
>>>    +    **bpftool map freeze**  *MAP*
>>> +          Freeze the map as read-only from user space. Entries from a
>>> +          frozen map can not longer be updated or deleted with the
>>> +          **bpf\ ()** system call. This operation is not reversible,
>>> +          and the map remains immutable from user space until its
>>> +          destruction. However, read and write permissions for BPF
>>> +          programs to the map remain unchanged.
>>
>> That is not correct, programs that are loaded into the system /after/
>> the map
>> has been frozen cannot modify values either, thus read-only from both
>> sides.
> 
> Are you entirely sure about it? I could not find the relevant
> restriction in the code, the checks seem to be on map flags
> (BPF_F_RDONLY) which do not seem to be modified by the "frozen" status
> in map_freeze()? And tests I ran on my side seem to indicate the map can
> still be updated by new programs. Did I miss something?
> 
> Tested on 5.3.0-rc1:
> 
> 1. Create hash map
> 2. Load BPF program foo, using map
> 3. Test-run program foo - map is updated
> 4. Freeze map - update effectively becomes impossible from user space
> 5. Load BPF program bar, using same map
> 6. Test-run program bar - map is still updated

Looks like I need some more coffee. ;-) Indeed, the program side was via
BPF_F_RDONLY_PROG flag.

^ 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