Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next 7/7] net/rds: Initialize ic->i_fastreg_wrs upon allocation
From: santosh.shilimkar @ 2019-07-01 20:15 UTC (permalink / raw)
  To: Gerd Rausch, netdev; +Cc: David Miller
In-Reply-To: <a97a92d4-c443-12e1-6d82-1646f9584828@oracle.com>

On 7/1/19 9:40 AM, Gerd Rausch wrote:
> Otherwise, if an IB connection is torn down before "rds_ib_setup_qp"
> is called, the value of "ic->i_fastreg_wrs" is still at zero
> (as it wasn't initialized by "rds_ib_setup_qp").
> Consequently "rds_ib_conn_path_shutdown" will spin forever,
> waiting for it to go back to "RDS_IB_DEFAULT_FR_WR",
> which of course will never happen as there are no
> outstanding work requests.
> 
> Signed-off-by: Gerd Rausch <gerd.rausch@oracle.com>
> ---
Looks good to me. Will add this to other fixes.


^ permalink raw reply

* Re: [PATCH net-next 6/7] net/rds: Keep track of and wait for FRWR segments in use upon shutdown
From: santosh.shilimkar @ 2019-07-01 20:15 UTC (permalink / raw)
  To: Gerd Rausch, netdev; +Cc: David Miller
In-Reply-To: <ad27abb5-b86a-1942-e2c8-2cba00812849@oracle.com>

On 7/1/19 9:40 AM, Gerd Rausch wrote:
> Since "rds_ib_free_frmr" and "rds_ib_free_frmr_list" simply put
> the FRMR memory segments on the "drop_list" or "free_list",
> and it is the job of "rds_ib_flush_mr_pool" to reap those entries
> by ultimately issuing a "IB_WR_LOCAL_INV" work-request,
> we need to trigger and then wait for all those memory segments
> attached to a particular connection to be fully released before
> we can move on to release the QP, CQ, etc.
> 
> So we make "rds_ib_conn_path_shutdown" wait for one more
> atomic_t called "i_fastreg_inuse_count" that keeps track of how
> many FRWR memory segments are out there marked "FRMR_IS_INUSE"
> (and also wake_up rds_ib_ring_empty_wait, as they go away).
> 
> Signed-off-by: Gerd Rausch <gerd.rausch@oracle.com>
> ---
Looks good to me. Will add this to other fixes.


^ permalink raw reply

* Re: [PATCH net-next 5/7] net/rds: Set fr_state only to FRMR_IS_FREE if IB_WR_LOCAL_INV had been successful
From: santosh.shilimkar @ 2019-07-01 20:14 UTC (permalink / raw)
  To: Gerd Rausch, netdev; +Cc: David Miller
In-Reply-To: <eb94e6bf-cbde-8cf1-b139-66fc8351f181@oracle.com>

On 7/1/19 9:40 AM, Gerd Rausch wrote:
> Fix a bug where fr_state first goes to FRMR_IS_STALE, because of a failure
> of operation IB_WR_LOCAL_INV, but then gets set back to "FRMR_IS_FREE"
> uncoditionally, even though the operation failed.
> 
> Signed-off-by: Gerd Rausch <gerd.rausch@oracle.com>
> ---
>   net/rds/ib_frmr.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/rds/ib_frmr.c b/net/rds/ib_frmr.c
> index 3c953034dca3..a5d8f4128515 100644
> --- a/net/rds/ib_frmr.c
> +++ b/net/rds/ib_frmr.c
> @@ -328,7 +328,8 @@ void rds_ib_mr_cqe_handler(struct rds_ib_connection *ic, struct ib_wc *wc)
>   	}
>   
>   	if (frmr->fr_inv) {
> -		frmr->fr_state = FRMR_IS_FREE;
> +		if (frmr->fr_state == FRMR_IS_INUSE)
> +			frmr->fr_state = FRMR_IS_FREE;
>   		frmr->fr_inv = false;
>   		wake_up(&frmr->fr_inv_done);
>   	}
> 
Looks good to me. Will add this to other fixes.

^ permalink raw reply

* Re: [PATCH 2/3] net: phy: realtek: Enable accessing RTL8211E extension pages
From: Andrew Lunn @ 2019-07-01 20:02 UTC (permalink / raw)
  To: Matthias Kaehlcke
  Cc: David S . Miller, Rob Herring, Mark Rutland, Florian Fainelli,
	Heiner Kallweit, netdev, devicetree, linux-kernel,
	Douglas Anderson
In-Reply-To: <20190701195225.120808-2-mka@chromium.org>

On Mon, Jul 01, 2019 at 12:52:24PM -0700, Matthias Kaehlcke wrote:
> The RTL8211E has extension pages, which can be accessed after
> selecting a page through a custom method. Add a function to
> modify bits in a register of an extension page and a few
> helpers for dealing with ext pages.
> 
> rtl8211e_modify_ext_paged() and rtl821e_restore_page() are
> inspired by their counterparts phy_modify_paged() and
> phy_restore_page().

Hi Matthias

While an extended page is selected, what happens to the normal
registers in the range 0-0x1c? Are they still accessible?

	  Andrew

^ permalink raw reply

* Re: [PATCH 2/2] samples: pktgen: allow to specify destination port
From: Daniel T. Lee @ 2019-07-01 20:01 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: David S . Miller, netdev, Robert Olsson, Jean Hsiao
In-Reply-To: <20190701170819.548a7457@carbon>

Thanks for the review!

About the equivalent port in the same burst thing, I didn't realize it
would work in
that way. It doesn't matter in my use-case, but thank you for letting me know!

On Tue, Jul 2, 2019 at 12:08 AM Jesper Dangaard Brouer
<brouer@redhat.com> wrote:
>
> On Sat, 29 Jun 2019 22:33:58 +0900
> "Daniel T. Lee" <danieltimlee@gmail.com> wrote:
>
> > Currently, kernel pktgen has the feature to specify udp destination port
> > for sending packet. (e.g. pgset "udp_dst_min 9")
> >
> > But on samples, each of the scripts doesn't have any option to achieve this.
> >
> > This commit adds the DST_PORT option to specify the target port(s) in the script.
> >
> >     -p : ($DST_PORT)  destination PORT range (e.g. 433-444) is also allowed
> >
> > Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com>
>
> Nice feature, this look very usable for testing.  I think my QA asked
> me for something similar.
>
> One nitpick is that script named pktgen_sample03_burst_single_flow.sh
> implies this is a single flow, but by specifying a port-range this will
> be more flows.  I'm okay with adding this, as the end-user specifying a
> port-range should realize this.  Thus, you get my ACK.
>
> Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
>
> Another thing you should realize (but you/we cannot do anything about)
> is that when the scripts use burst or clone, then the port (UDPDST_RND)
> will be the same for all packets in the same burst.  I don't know if it
> matters for your use-case.
>
> --
> Best regards,
>   Jesper Dangaard Brouer
>   MSc.CS, Principal Kernel Engineer at Red Hat
>   LinkedIn: http://www.linkedin.com/in/brouer

^ permalink raw reply

* Re: [PATCH net-next] r8169: fix ntohs/htons sparse warnings
From: Al Viro @ 2019-07-01 19:56 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: Realtek linux nic maintainers, David Miller,
	netdev@vger.kernel.org
In-Reply-To: <1d1f9dba-1ade-7782-6cc0-3151a7086a4b@gmail.com>

On Mon, Jul 01, 2019 at 09:35:28PM +0200, Heiner Kallweit wrote:
> Sparse complains about casting to/from restricted __be16. Fix this.

Fix what, exactly?  Force-cast is not a fix - it's "STFU, I know
better, it's really correct" to sparse.  Which may or may not
match the reality, but it definitely requires more in way of
commit message than "sparse says it's wrong; shut it up".

>  static void rtl8169_rx_vlan_tag(struct RxDesc *desc, struct sk_buff *skb)
> @@ -1537,7 +1537,7 @@ static void rtl8169_rx_vlan_tag(struct RxDesc *desc, struct sk_buff *skb)
>  
>  	if (opts2 & RxVlanTag)
>  		__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q),
> -				       ntohs(opts2 & 0xffff));
> +				       ntohs((__force __be16)(opts2 & 0xffff)));
>  }

Should that be ntohs at all?  What behaviour is correct on big-endian host?

AFAICS, in that code opts2 comes from little-endian 32bit.  It's converted to
host-endian, lower 16 bits (i.e. the first two octets in memory) are then
fed to ntohs.  Suppose we had in-core value stored as A0, A1, A2, A3.
On little-endian that code will yield A0 * 256 + A1, treated as host-endian.
On big-endian the same will yield A1 * 256 + A0.  Is that actually correct?

The code dealing with the value passed to __vlan_hwaccel_put_tag() as the
third argument treats it as a host-endian integer.  So... Has anyone
tested that code on b-e host?  Should that ntohs() actually be swab16(),
yielding (on any host) the same value we currently get for l-e hosts only?

^ permalink raw reply

* [PATCH 1/3] dt-bindings: net: Add bindings for Realtek PHYs
From: Matthias Kaehlcke @ 2019-07-01 19:52 UTC (permalink / raw)
  To: David S . Miller, Rob Herring, Mark Rutland, Andrew Lunn,
	Florian Fainelli, Heiner Kallweit
  Cc: netdev, devicetree, linux-kernel, Douglas Anderson,
	Matthias Kaehlcke

Add the 'realtek,enable-ssc' property to enable Spread Spectrum
Clocking (SSC) on Realtek PHYs that support it.

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
 .../devicetree/bindings/net/realtek.txt       | 21 +++++++++++++++++++
 1 file changed, 21 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/realtek.txt

diff --git a/Documentation/devicetree/bindings/net/realtek.txt b/Documentation/devicetree/bindings/net/realtek.txt
new file mode 100644
index 000000000000..9fad97e7404f
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/realtek.txt
@@ -0,0 +1,21 @@
+Realtek PHY properties.
+
+This document describes properties of Realtek PHYs.
+
+Optional properties:
+- realtek,enable-ssc	Enable Spread Spectrum Clocking (SSC) on this port.
+			SSC is only available on some Realtek PHYs (e.g.
+			RTL8211E).
+
+Example:
+
+mdio0 {
+	compatible = "snps,dwmac-mdio";
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	ethphy: ethernet-phy@1 {
+		reg = <1>;
+		realtek,enable-ssc;
+	};
+};
-- 
2.22.0.410.gd8fdbe21b5-goog


^ permalink raw reply related

* [PATCH 3/3] net: phy: realtek: Support SSC for the RTL8211E
From: Matthias Kaehlcke @ 2019-07-01 19:52 UTC (permalink / raw)
  To: David S . Miller, Rob Herring, Mark Rutland, Andrew Lunn,
	Florian Fainelli, Heiner Kallweit
  Cc: netdev, devicetree, linux-kernel, Douglas Anderson,
	Matthias Kaehlcke
In-Reply-To: <20190701195225.120808-1-mka@chromium.org>

By default Spread-Spectrum Clocking (SSC) is disabled on the RTL8211E.
Enable it if the device tree property 'realtek,enable-ssc' exists.

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
 drivers/net/phy/realtek.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index dfc2e20ef335..b617169ccc8c 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -9,8 +9,10 @@
  * Copyright (c) 2004 Freescale Semiconductor, Inc.
  */
 #include <linux/bitops.h>
-#include <linux/phy.h>
+#include <linux/device.h>
+#include <linux/of.h>
 #include <linux/module.h>
+#include <linux/phy.h>
 
 #define RTL821x_PHYSR				0x11
 #define RTL821x_PHYSR_DUPLEX			BIT(13)
@@ -28,6 +30,8 @@
 
 #define RTL8211E_EXT_PAGE			7
 #define RTL8211E_EPAGSR				0x1e
+#define RTL8211E_SCR				0x1a
+#define RTL8211E_SCR_DISABLE_RXC_SSC		BIT(2)
 
 #define RTL8211F_INSR				0x1d
 
@@ -87,8 +91,8 @@ static int rtl821e_restore_page(struct phy_device *phydev, int oldpage, int ret)
 	return ret;
 }
 
-static int __maybe_unused rtl8211e_modify_ext_paged(struct phy_device *phydev,
-				    int page, u32 regnum, u16 mask, u16 set)
+static int rtl8211e_modify_ext_paged(struct phy_device *phydev, int page,
+				     u32 regnum, u16 mask, u16 set)
 {
 	int ret = 0;
 	int oldpage;
@@ -114,6 +118,22 @@ static int __maybe_unused rtl8211e_modify_ext_paged(struct phy_device *phydev,
 	return rtl821e_restore_page(phydev, oldpage, ret);
 }
 
+static int rtl8211e_probe(struct phy_device *phydev)
+{
+	struct device *dev = &phydev->mdio.dev;
+	int err;
+
+	if (of_property_read_bool(dev->of_node, "realtek,enable-ssc")) {
+		err = rtl8211e_modify_ext_paged(phydev, 0xa0, RTL8211E_SCR,
+						RTL8211E_SCR_DISABLE_RXC_SSC,
+						0);
+		if (err)
+			dev_err(dev, "failed to enable SSC on RXC: %d\n", err);
+	}
+
+	return 0;
+}
+
 static int rtl8201_ack_interrupt(struct phy_device *phydev)
 {
 	int err;
@@ -372,6 +392,7 @@ static struct phy_driver realtek_drvs[] = {
 		.config_init	= &rtl8211e_config_init,
 		.ack_interrupt	= &rtl821x_ack_interrupt,
 		.config_intr	= &rtl8211e_config_intr,
+		.probe          = rtl8211e_probe,
 		.suspend	= genphy_suspend,
 		.resume		= genphy_resume,
 		.read_page	= rtl821x_read_page,
-- 
2.22.0.410.gd8fdbe21b5-goog


^ permalink raw reply related

* [PATCH 2/3] net: phy: realtek: Enable accessing RTL8211E extension pages
From: Matthias Kaehlcke @ 2019-07-01 19:52 UTC (permalink / raw)
  To: David S . Miller, Rob Herring, Mark Rutland, Andrew Lunn,
	Florian Fainelli, Heiner Kallweit
  Cc: netdev, devicetree, linux-kernel, Douglas Anderson,
	Matthias Kaehlcke
In-Reply-To: <20190701195225.120808-1-mka@chromium.org>

The RTL8211E has extension pages, which can be accessed after
selecting a page through a custom method. Add a function to
modify bits in a register of an extension page and a few
helpers for dealing with ext pages.

rtl8211e_modify_ext_paged() and rtl821e_restore_page() are
inspired by their counterparts phy_modify_paged() and
phy_restore_page().

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
This code might be applicable to other Realtek PHYs, but I don't
have access to the datasheets to confirm it, so for now it's just
for the RTL8211E.

 drivers/net/phy/realtek.c | 61 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index a669945eb829..dfc2e20ef335 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -26,6 +26,9 @@
 #define RTL821x_EXT_PAGE_SELECT			0x1e
 #define RTL821x_PAGE_SELECT			0x1f
 
+#define RTL8211E_EXT_PAGE			7
+#define RTL8211E_EPAGSR				0x1e
+
 #define RTL8211F_INSR				0x1d
 
 #define RTL8211F_TX_DELAY			BIT(8)
@@ -53,6 +56,64 @@ static int rtl821x_write_page(struct phy_device *phydev, int page)
 	return __phy_write(phydev, RTL821x_PAGE_SELECT, page);
 }
 
+static int rtl821e_select_ext_page(struct phy_device *phydev, int page)
+{
+	int rc;
+
+	rc = phy_write(phydev, RTL821x_PAGE_SELECT, RTL8211E_EXT_PAGE);
+	if (rc)
+		return rc;
+
+	return phy_write(phydev, RTL8211E_EPAGSR, page);
+}
+
+static int rtl821e_restore_page(struct phy_device *phydev, int oldpage, int ret)
+{
+	int r;
+
+	if (oldpage >= 0) {
+		r = phy_write(phydev, RTL821x_PAGE_SELECT, oldpage);
+
+		/* Propagate the operation return code if the page write
+		 * was successful.
+		 */
+		if (ret >= 0 && r < 0)
+			ret = r;
+	} else {
+		/* Propagate the page selection error code */
+		ret = oldpage;
+	}
+
+	return ret;
+}
+
+static int __maybe_unused rtl8211e_modify_ext_paged(struct phy_device *phydev,
+				    int page, u32 regnum, u16 mask, u16 set)
+{
+	int ret = 0;
+	int oldpage;
+	int new;
+
+	oldpage = phy_read(phydev, RTL821x_PAGE_SELECT);
+	if (oldpage < 0)
+		goto out;
+
+	ret = rtl821e_select_ext_page(phydev, page);
+	if (ret)
+		goto out;
+
+	ret = phy_read(phydev, regnum);
+	if (ret < 0)
+		goto out;
+
+	new = (ret & ~mask) | set;
+	if (new != ret)
+		ret = phy_write(phydev, regnum, new);
+
+out:
+	return rtl821e_restore_page(phydev, oldpage, ret);
+}
+
 static int rtl8201_ack_interrupt(struct phy_device *phydev)
 {
 	int err;
-- 
2.22.0.410.gd8fdbe21b5-goog


^ permalink raw reply related

* [PATCH net-next] r8169: fix ntohs/htons sparse warnings
From: Heiner Kallweit @ 2019-07-01 19:35 UTC (permalink / raw)
  To: Realtek linux nic maintainers, David Miller; +Cc: netdev@vger.kernel.org

Sparse complains about casting to/from restricted __be16. Fix this.

Fixes: 759d09574172 ("r8169: improve handling VLAN tag")
Reported-by: kbuild test robot <lkp@intel.com>
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/ethernet/realtek/r8169_main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index a73f25321..450c74dc1 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -1528,7 +1528,7 @@ static int rtl8169_set_features(struct net_device *dev,
 static inline u32 rtl8169_tx_vlan_tag(struct sk_buff *skb)
 {
 	return (skb_vlan_tag_present(skb)) ?
-		TxVlanTag | htons(skb_vlan_tag_get(skb)) : 0x00;
+		TxVlanTag | (__force u16)htons(skb_vlan_tag_get(skb)) : 0x00;
 }
 
 static void rtl8169_rx_vlan_tag(struct RxDesc *desc, struct sk_buff *skb)
@@ -1537,7 +1537,7 @@ static void rtl8169_rx_vlan_tag(struct RxDesc *desc, struct sk_buff *skb)
 
 	if (opts2 & RxVlanTag)
 		__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q),
-				       ntohs(opts2 & 0xffff));
+				       ntohs((__force __be16)(opts2 & 0xffff)));
 }
 
 static void rtl8169_get_regs(struct net_device *dev, struct ethtool_regs *regs,
-- 
2.22.0


^ permalink raw reply related

* Re: [PATCH net-next] bonding/main: fix NULL dereference in bond_select_active_slave()
From: Eric Dumazet @ 2019-07-01 19:31 UTC (permalink / raw)
  To: Jay Vosburgh
  Cc: David S . Miller, netdev, Eric Dumazet, John Sperbeck,
	Jarod Wilson, Veaceslav Falico, Andy Gospodarek
In-Reply-To: <18508.1562008524@famine>

On Mon, Jul 1, 2019 at 9:15 PM Jay Vosburgh <jay.vosburgh@canonical.com> wrote:
>
> Eric Dumazet <edumazet@google.com> wrote:
>
> >A bonding master can be up while best_slave is NULL.
> >
> >[12105.636318] BUG: unable to handle kernel NULL pointer dereference at 0000000000000000
> >[12105.638204] mlx4_en: eth1: Linkstate event 1 -> 1
> >[12105.648984] IP: bond_select_active_slave+0x125/0x250
> >[12105.653977] PGD 0 P4D 0
> >[12105.656572] Oops: 0000 [#1] SMP PTI
> >[12105.660487] gsmi: Log Shutdown Reason 0x03
> >[12105.664620] Modules linked in: kvm_intel loop act_mirred uhaul vfat fat stg_standard_ftl stg_megablocks stg_idt stg_hdi stg elephant_dev_num stg_idt_eeprom w1_therm wire i2c_mux_pca954x i2c_mux mlx4_i2c i2c_usb cdc_acm ehci_pci ehci_hcd i2c_iimc mlx4_en mlx4_ib ib_uverbs ib_core mlx4_core [last unloaded: kvm_intel]
> >[12105.685686] mlx4_core 0000:03:00.0: dispatching link up event for port 2
> >[12105.685700] mlx4_en: eth2: Linkstate event 2 -> 1
> >[12105.685700] mlx4_en: eth2: Link Up (linkstate)
> >[12105.724452] Workqueue: bond0 bond_mii_monitor
> >[12105.728854] RIP: 0010:bond_select_active_slave+0x125/0x250
> >[12105.734355] RSP: 0018:ffffaf146a81fd88 EFLAGS: 00010246
> >[12105.739637] RAX: 0000000000000003 RBX: ffff8c62b03c6900 RCX: 0000000000000000
> >[12105.746838] RDX: 0000000000000000 RSI: ffffaf146a81fd08 RDI: ffff8c62b03c6000
> >[12105.754054] RBP: ffffaf146a81fdb8 R08: 0000000000000001 R09: ffff8c517d387600
> >[12105.761299] R10: 00000000001075d9 R11: ffffffffaceba92f R12: 0000000000000000
> >[12105.768553] R13: ffff8c8240ae4800 R14: 0000000000000000 R15: 0000000000000000
> >[12105.775748] FS:  0000000000000000(0000) GS:ffff8c62bfa40000(0000) knlGS:0000000000000000
> >[12105.783892] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> >[12105.789716] CR2: 0000000000000000 CR3: 0000000d0520e001 CR4: 00000000001626f0
> >[12105.796976] Call Trace:
> >[12105.799446]  [<ffffffffac31d387>] bond_mii_monitor+0x497/0x6f0
> >[12105.805317]  [<ffffffffabd42643>] process_one_work+0x143/0x370
> >[12105.811225]  [<ffffffffabd42c7a>] worker_thread+0x4a/0x360
> >[12105.816761]  [<ffffffffabd48bc5>] kthread+0x105/0x140
> >[12105.821865]  [<ffffffffabd42c30>] ? rescuer_thread+0x380/0x380
> >[12105.827757]  [<ffffffffabd48ac0>] ? kthread_associate_blkcg+0xc0/0xc0
> >[12105.834266]  [<ffffffffac600241>] ret_from_fork+0x51/0x60
> >
> >Fixes: e2a7420df2e0 ("bonding/main: convert to using slave printk macros")
> >Signed-off-by: Eric Dumazet <edumazet@google.com>
> >Reported-by: John Sperbeck <jsperbeck@google.com>
> >Cc: Jarod Wilson <jarod@redhat.com>
> >CC: Jay Vosburgh <j.vosburgh@gmail.com>
> >CC: Veaceslav Falico <vfalico@gmail.com>
> >CC: Andy Gospodarek <andy@greyhouse.net>
> >---
> > drivers/net/bonding/bond_main.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> >index a30595955a37a485b9e045a31969313f8336b668..84168455aded96dfd85b310841dee2a0d917b580 100644
> >--- a/drivers/net/bonding/bond_main.c
> >+++ b/drivers/net/bonding/bond_main.c
> >@@ -937,7 +937,7 @@ void bond_select_active_slave(struct bonding *bond)
> >                       return;
> >
> >               if (netif_carrier_ok(bond->dev))
> >-                      slave_info(bond->dev, best_slave->dev, "active interface up!\n");
> >+                      netdev_info(bond->dev, "active interface up!\n");
> >               else
> >                       netdev_info(bond->dev, "now running without any active interface!\n");
> >       }
>
>         What is the bonding mode and options in the failure case?

I am not sure.

Basically I only have crash dumps at this point (no idea how to repro the bug)

I guess that is :

Bonding Mode: IEEE 802.3ad Dynamic link aggregation
Transmit Hash Policy: encap3+4 (4)
Use Carrier: 1
MII Status: up
MII Polling Interval (ms): 100
Up Delay (ms): 0
Down Delay (ms): 0
MAC address: 00:1a:11:c0:65:40
Active Members: eth1 eth2


>
>         I see that the fix is fine in the sense that it returns to the
> original status quo for the message.
>
>         However, the code path seems odd; if best_slave is NULL, that
> means that bond_find_best_slave() saw all slaves as down, but if
> netif_carrier_ok is true, then bond_set_carrier() did not.

^ permalink raw reply

* Re: [PATCH net-next] bonding/main: fix NULL dereference in bond_select_active_slave()
From: Jay Vosburgh @ 2019-07-01 19:15 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David S . Miller, netdev, Eric Dumazet, John Sperbeck,
	Jarod Wilson, Veaceslav Falico, Andy Gospodarek
In-Reply-To: <20190701174851.70293-1-edumazet@google.com>

Eric Dumazet <edumazet@google.com> wrote:

>A bonding master can be up while best_slave is NULL.
>
>[12105.636318] BUG: unable to handle kernel NULL pointer dereference at 0000000000000000
>[12105.638204] mlx4_en: eth1: Linkstate event 1 -> 1
>[12105.648984] IP: bond_select_active_slave+0x125/0x250
>[12105.653977] PGD 0 P4D 0
>[12105.656572] Oops: 0000 [#1] SMP PTI
>[12105.660487] gsmi: Log Shutdown Reason 0x03
>[12105.664620] Modules linked in: kvm_intel loop act_mirred uhaul vfat fat stg_standard_ftl stg_megablocks stg_idt stg_hdi stg elephant_dev_num stg_idt_eeprom w1_therm wire i2c_mux_pca954x i2c_mux mlx4_i2c i2c_usb cdc_acm ehci_pci ehci_hcd i2c_iimc mlx4_en mlx4_ib ib_uverbs ib_core mlx4_core [last unloaded: kvm_intel]
>[12105.685686] mlx4_core 0000:03:00.0: dispatching link up event for port 2
>[12105.685700] mlx4_en: eth2: Linkstate event 2 -> 1
>[12105.685700] mlx4_en: eth2: Link Up (linkstate)
>[12105.724452] Workqueue: bond0 bond_mii_monitor
>[12105.728854] RIP: 0010:bond_select_active_slave+0x125/0x250
>[12105.734355] RSP: 0018:ffffaf146a81fd88 EFLAGS: 00010246
>[12105.739637] RAX: 0000000000000003 RBX: ffff8c62b03c6900 RCX: 0000000000000000
>[12105.746838] RDX: 0000000000000000 RSI: ffffaf146a81fd08 RDI: ffff8c62b03c6000
>[12105.754054] RBP: ffffaf146a81fdb8 R08: 0000000000000001 R09: ffff8c517d387600
>[12105.761299] R10: 00000000001075d9 R11: ffffffffaceba92f R12: 0000000000000000
>[12105.768553] R13: ffff8c8240ae4800 R14: 0000000000000000 R15: 0000000000000000
>[12105.775748] FS:  0000000000000000(0000) GS:ffff8c62bfa40000(0000) knlGS:0000000000000000
>[12105.783892] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>[12105.789716] CR2: 0000000000000000 CR3: 0000000d0520e001 CR4: 00000000001626f0
>[12105.796976] Call Trace:
>[12105.799446]  [<ffffffffac31d387>] bond_mii_monitor+0x497/0x6f0
>[12105.805317]  [<ffffffffabd42643>] process_one_work+0x143/0x370
>[12105.811225]  [<ffffffffabd42c7a>] worker_thread+0x4a/0x360
>[12105.816761]  [<ffffffffabd48bc5>] kthread+0x105/0x140
>[12105.821865]  [<ffffffffabd42c30>] ? rescuer_thread+0x380/0x380
>[12105.827757]  [<ffffffffabd48ac0>] ? kthread_associate_blkcg+0xc0/0xc0
>[12105.834266]  [<ffffffffac600241>] ret_from_fork+0x51/0x60
>
>Fixes: e2a7420df2e0 ("bonding/main: convert to using slave printk macros")
>Signed-off-by: Eric Dumazet <edumazet@google.com>
>Reported-by: John Sperbeck <jsperbeck@google.com>
>Cc: Jarod Wilson <jarod@redhat.com>
>CC: Jay Vosburgh <j.vosburgh@gmail.com>
>CC: Veaceslav Falico <vfalico@gmail.com>
>CC: Andy Gospodarek <andy@greyhouse.net>
>---
> drivers/net/bonding/bond_main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index a30595955a37a485b9e045a31969313f8336b668..84168455aded96dfd85b310841dee2a0d917b580 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -937,7 +937,7 @@ void bond_select_active_slave(struct bonding *bond)
> 			return;
> 
> 		if (netif_carrier_ok(bond->dev))
>-			slave_info(bond->dev, best_slave->dev, "active interface up!\n");
>+			netdev_info(bond->dev, "active interface up!\n");
> 		else
> 			netdev_info(bond->dev, "now running without any active interface!\n");
> 	}

	What is the bonding mode and options in the failure case?

	I see that the fix is fine in the sense that it returns to the
original status quo for the message.

	However, the code path seems odd; if best_slave is NULL, that
means that bond_find_best_slave() saw all slaves as down, but if
netif_carrier_ok is true, then bond_set_carrier() did not.

	-J

---
	-Jay Vosburgh, jay.vosburgh@canonical.com

^ permalink raw reply

* Re: [PATCH net-next 5/5] ipv4: use indirect call wrappers for {tcp,udp}_{recv,send}msg()
From: Willem de Bruijn @ 2019-07-01 19:07 UTC (permalink / raw)
  To: Paolo Abeni; +Cc: Network Development, David S. Miller
In-Reply-To: <8c32b92eee12bf0725ead331e7607d8c4012d51f.1561999976.git.pabeni@redhat.com>

On Mon, Jul 1, 2019 at 1:10 PM Paolo Abeni <pabeni@redhat.com> wrote:
>
> This avoids an indirect call per syscall for common ipv4 transports
>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
>  net/ipv4/af_inet.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> index 8421e2f5bbb3..9a2f17d0c5f5 100644
> --- a/net/ipv4/af_inet.c
> +++ b/net/ipv4/af_inet.c
> @@ -797,6 +797,8 @@ int inet_send_prepare(struct sock *sk)
>  }
>  EXPORT_SYMBOL_GPL(inet_send_prepare);
>
> +INDIRECT_CALLABLE_DECLARE(int udp_sendmsg(struct sock *, struct msghdr *,
> +                                         size_t));

Small nit: this is already defined in include/net/udp.h, which is
included. So like tcp_sendmsg, probably no need to declare.

If defining inet6_sendmsg and inet6_recvmsg in include/net/ipv6.h,
perhaps do the same for the other missing functions, instead of these
indirect declarations at the callsite?

Aside from that small point, patch set looks great to me. Thanks Paolo.

Acked-by: Willem de Bruijn <willemb@google.com>

^ permalink raw reply

* Re: r8169 not working on 5.2.0rc6 with GPD MicroPC
From: Karsten Wiborg @ 2019-07-01 19:07 UTC (permalink / raw)
  To: Heiner Kallweit, Andrew Lunn; +Cc: nic_swsd, romieu, netdev
In-Reply-To: <d48eb3dd-be07-1422-4649-91f3461676c4@gmail.com>

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

Hi Heiner,

that is very unfortunately regarding the state of the device. I guess
GPD won't really react. Anyway thank you very much for trying to contact
them.
Thanks also a lot for your help and input.

Best regards,
Karsten

On 01/07/2019 20:51, Heiner Kallweit wrote:
> On 01.07.2019 20:15, Karsten Wiborg wrote:
> The information is sufficient now. Still, using a random MAC address
> is an emergency fallback. The device is simply broken.
> I contacted GPD, let's see whether they respond something.
> 
> In parallel I'll add a random MAC address as fallback to the
> mainline driver.


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4156 bytes --]

^ permalink raw reply

* Re: r8169 not working on 5.2.0rc6 with GPD MicroPC
From: Heiner Kallweit @ 2019-07-01 18:51 UTC (permalink / raw)
  To: Karsten Wiborg, Andrew Lunn; +Cc: nic_swsd, romieu, netdev
In-Reply-To: <672d3b3f-e55d-e2bb-1d8c-a83d0a0c057a@web.de>

On 01.07.2019 20:15, Karsten Wiborg wrote:
> Hi Andrew, Heiner,
> 
> the device is a really small notebook. So detaching mains still leaves
> the battery which is delicately built in. So can't currently remove
> power completely.
> 
> Anyway can I deliver more debugging data to you guys which might add
> support for the r8169 for this device?
> 
The information is sufficient now. Still, using a random MAC address
is an emergency fallback. The device is simply broken.
I contacted GPD, let's see whether they respond something.

In parallel I'll add a random MAC address as fallback to the
mainline driver.

> Regards,
> Karsten
> 
Heiner

> On 01/07/2019 15:35, Andrew Lunn wrote:
>>> When the vendor driver assigns a random MAC address, it writes it to the
>>> chip. The related registers may be persistent (can't say exactly due to
>>> missing documentation).
>>
>> If the device supports WOL, it could be it is powered using the
>> standby supply, not the main supply. Try pulling the plug from the
>> wall to really remove all power.
>>
>>      Andrew
>>
> 


^ permalink raw reply

* Re: [PATCH bpf] selftests: bpf: fix inlines in test_lwt_seg6local
From: Y Song @ 2019-07-01 18:39 UTC (permalink / raw)
  To: Song Liu
  Cc: Jiri Benc, Networking, bpf, Alexei Starovoitov, Daniel Borkmann,
	Mathieu Xhonneux
In-Reply-To: <CAPhsuW4Ric_nMGxpKf3mEJw3JDBZYpbeAQwTW_Nrsz79T2zisw@mail.gmail.com>

On Sat, Jun 29, 2019 at 11:05 AM Song Liu <liu.song.a23@gmail.com> wrote:
>
> On Sat, Jun 29, 2019 at 11:04 AM Song Liu <liu.song.a23@gmail.com> wrote:
> >
> > On Sat, Jun 29, 2019 at 7:43 AM Jiri Benc <jbenc@redhat.com> wrote:
> > >
> > > Selftests are reporting this failure in test_lwt_seg6local.sh:
> > >
> > > + ip netns exec ns2 ip -6 route add fb00::6 encap bpf in obj test_lwt_seg6local.o sec encap_srh dev veth2
> > > Error fetching program/map!
> > > Failed to parse eBPF program: Operation not permitted
> > >
> > > The problem is __attribute__((always_inline)) alone is not enough to prevent
> > > clang from inserting those functions in .text. In that case, .text is not
> > > marked as relocateable.
> > >
> > > See the output of objdump -h test_lwt_seg6local.o:
> > >
> > > Idx Name          Size      VMA               LMA               File off  Algn
> > >   0 .text         00003530  0000000000000000  0000000000000000  00000040  2**3
> > >                   CONTENTS, ALLOC, LOAD, READONLY, CODE
> > >
> > > This causes the iproute bpf loader to fail in bpf_fetch_prog_sec:
> > > bpf_has_call_data returns true but bpf_fetch_prog_relo fails as there's no
> > > relocateable .text section in the file.
> > >
> > > Add 'static inline' to fix this.
> > >
> > > Fixes: c99a84eac026 ("selftests/bpf: test for seg6local End.BPF action")
> > > Signed-off-by: Jiri Benc <jbenc@redhat.com>
> >
> > Maybe use "__always_inline" as most other tests do?
>
> I meant "static __always_inline".

By default, we have
# define __always_inline        inline __attribute__((always_inline))

So just use __always_inline should be less verbose in your patch.

BTW, what compiler did you use have this behavior?
Did you have issues with `static __attribute__((always_inline))`?

>
> Song

^ permalink raw reply

* Re: [PATCH bpf-next 1/2] bpf: allow wide (u64) aligned stores for some fields of bpf_sock_addr
From: Stanislav Fomichev @ 2019-07-01 18:38 UTC (permalink / raw)
  To: Yonghong Song
  Cc: Andrii Nakryiko, Stanislav Fomichev, netdev@vger.kernel.org,
	bpf@vger.kernel.org, davem@davemloft.net, ast@kernel.org,
	daniel@iogearbox.net, Andrii Nakryiko, kernel test robot
In-Reply-To: <a66c937f-94c0-eaf8-5b37-8587d66c0c62@fb.com>

On 07/01, Yonghong Song wrote:
> 
> 
> On 7/1/19 9:04 AM, Stanislav Fomichev wrote:
> > On 07/01, Andrii Nakryiko wrote:
> >> On Sat, Jun 29, 2019 at 10:53 PM Yonghong Song <yhs@fb.com> wrote:
> >>>
> >>>
> >>>
> >>> On 6/28/19 4:10 PM, Stanislav Fomichev wrote:
> >>>> Since commit cd17d7770578 ("bpf/tools: sync bpf.h") clang decided
> >>>> that it can do a single u64 store into user_ip6[2] instead of two
> >>>> separate u32 ones:
> >>>>
> >>>>    #  17: (18) r2 = 0x100000000000000
> >>>>    #  ; ctx->user_ip6[2] = bpf_htonl(DST_REWRITE_IP6_2);
> >>>>    #  19: (7b) *(u64 *)(r1 +16) = r2
> >>>>    #  invalid bpf_context access off=16 size=8
> >>>>
> >>>>   From the compiler point of view it does look like a correct thing
> >>>> to do, so let's support it on the kernel side.
> >>>>
> >>>> Credit to Andrii Nakryiko for a proper implementation of
> >>>> bpf_ctx_wide_store_ok.
> >>>>
> >>>> Cc: Andrii Nakryiko <andriin@fb.com>
> >>>> Cc: Yonghong Song <yhs@fb.com>
> >>>> Fixes: cd17d7770578 ("bpf/tools: sync bpf.h")
> >>>> Reported-by: kernel test robot <rong.a.chen@intel.com>
> >>>> Signed-off-by: Stanislav Fomichev <sdf@google.com>
> >>>
> >>> The change looks good to me with the following nits:
> >>>     1. could you add a cover letter for the patch set?
> >>>        typically if the number of patches is more than one,
> >>>        it would be a good practice with a cover letter.
> >>>        See bpf_devel_QA.rst .
> >>>     2. with this change, the comments in uapi bpf.h
> >>>        are not accurate any more.
> >>>           __u32 user_ip6[4];      /* Allows 1,2,4-byte read an 4-byte write.
> >>>                                    * Stored in network byte order.
> >>>
> >>>                                    */
> >>>           __u32 msg_src_ip6[4];   /* Allows 1,2,4-byte read an 4-byte write.
> >>>                                    * Stored in network byte order.
> >>>                                    */
> >>>        now for stores, aligned 8-byte write is permitted.
> >>>        could you update this as well?
> >>>
> >>>   From the typical usage pattern, I did not see a need
> >>> for 8-tye read of user_ip6 and msg_src_ip6 yet. So let
> >>> us just deal with write for now.
> >>
> >> But I guess it's still possible for clang to optimize two consecutive
> >> 4-byte reads into single 8-byte read in some circumstances? If that's
> >> the case, maybe it's a good idea to have corresponding read checks as
> >> well?
> > I guess clang can do those kinds of optimizations. I can put it on my
> > todo and address later (or when we actually see it out in the wild).
> 
> Okay, I find a Facebook internal app. does trying to read the 4 bytes
> and compare to a predefined loopback address. We may need to handle
> read cases as well. But this can be a followup after actual tryout.
Sounds good, will follow up on that.

> > 
> >> But overall this looks good to me:
> >>
> >> Acked-by: Andrii Nakryiko <andriin@fb.com>
> > Thanks for a review!
> > 
> >>>
> >>> With the above two nits,
> >>> Acked-by: Yonghong Song <yhs@fb.com>
> >>>
> >>>> ---
> >>>>    include/linux/filter.h |  6 ++++++
> >>>>    net/core/filter.c      | 22 ++++++++++++++--------
> >>>>    2 files changed, 20 insertions(+), 8 deletions(-)
> >>>>
> >>>> diff --git a/include/linux/filter.h b/include/linux/filter.h
> >>>> index 340f7d648974..3901007e36f1 100644
> >>>> --- a/include/linux/filter.h
> >>>> +++ b/include/linux/filter.h
> >>>> @@ -746,6 +746,12 @@ bpf_ctx_narrow_access_ok(u32 off, u32 size, u32 size_default)
> >>>>        return size <= size_default && (size & (size - 1)) == 0;
> >>>>    }
> >>>>
> >>>> +#define bpf_ctx_wide_store_ok(off, size, type, field)                        \
> >>>> +     (size == sizeof(__u64) &&                                       \
> >>>> +     off >= offsetof(type, field) &&                                 \
> >>>> +     off + sizeof(__u64) <= offsetofend(type, field) &&              \
> >>>> +     off % sizeof(__u64) == 0)
> >>>> +
> >>>>    #define bpf_classic_proglen(fprog) (fprog->len * sizeof(fprog->filter[0]))
> >>>>
> >>>>    static inline void bpf_prog_lock_ro(struct bpf_prog *fp)
> >>>> diff --git a/net/core/filter.c b/net/core/filter.c
> >>>> index dc8534be12fc..5d33f2146dab 100644
> >>>> --- a/net/core/filter.c
> >>>> +++ b/net/core/filter.c
> >>>> @@ -6849,6 +6849,16 @@ static bool sock_addr_is_valid_access(int off, int size,
> >>>>                        if (!bpf_ctx_narrow_access_ok(off, size, size_default))
> >>>>                                return false;
> >>>>                } else {
> >>>> +                     if (bpf_ctx_wide_store_ok(off, size,
> >>>> +                                               struct bpf_sock_addr,
> >>>> +                                               user_ip6))
> >>>> +                             return true;
> >>>> +
> >>>> +                     if (bpf_ctx_wide_store_ok(off, size,
> >>>> +                                               struct bpf_sock_addr,
> >>>> +                                               msg_src_ip6))
> >>>> +                             return true;
> >>>> +
> >>>>                        if (size != size_default)
> >>>>                                return false;
> >>>>                }
> >>>> @@ -7689,9 +7699,6 @@ static u32 xdp_convert_ctx_access(enum bpf_access_type type,
> >>>>    /* SOCK_ADDR_STORE_NESTED_FIELD_OFF() has semantic similar to
> >>>>     * SOCK_ADDR_LOAD_NESTED_FIELD_SIZE_OFF() but for store operation.
> >>>>     *
> >>>> - * It doesn't support SIZE argument though since narrow stores are not
> >>>> - * supported for now.
> >>>> - *
> >>>>     * In addition it uses Temporary Field TF (member of struct S) as the 3rd
> >>>>     * "register" since two registers available in convert_ctx_access are not
> >>>>     * enough: we can't override neither SRC, since it contains value to store, nor
> >>>> @@ -7699,7 +7706,7 @@ static u32 xdp_convert_ctx_access(enum bpf_access_type type,
> >>>>     * instructions. But we need a temporary place to save pointer to nested
> >>>>     * structure whose field we want to store to.
> >>>>     */
> >>>> -#define SOCK_ADDR_STORE_NESTED_FIELD_OFF(S, NS, F, NF, OFF, TF)                     \
> >>>> +#define SOCK_ADDR_STORE_NESTED_FIELD_OFF(S, NS, F, NF, SIZE, OFF, TF)               \
> >>>>        do {                                                                   \
> >>>>                int tmp_reg = BPF_REG_9;                                       \
> >>>>                if (si->src_reg == tmp_reg || si->dst_reg == tmp_reg)          \
> >>>> @@ -7710,8 +7717,7 @@ static u32 xdp_convert_ctx_access(enum bpf_access_type type,
> >>>>                                      offsetof(S, TF));                        \
> >>>>                *insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(S, F), tmp_reg,         \
> >>>>                                      si->dst_reg, offsetof(S, F));            \
> >>>> -             *insn++ = BPF_STX_MEM(                                         \
> >>>> -                     BPF_FIELD_SIZEOF(NS, NF), tmp_reg, si->src_reg,        \
> >>>> +             *insn++ = BPF_STX_MEM(SIZE, tmp_reg, si->src_reg,              \
> >>>>                        bpf_target_off(NS, NF, FIELD_SIZEOF(NS, NF),           \
> >>>>                                       target_size)                            \
> >>>>                                + OFF);                                        \
> >>>> @@ -7723,8 +7729,8 @@ static u32 xdp_convert_ctx_access(enum bpf_access_type type,
> >>>>                                                      TF)                      \
> >>>>        do {                                                                   \
> >>>>                if (type == BPF_WRITE) {                                       \
> >>>> -                     SOCK_ADDR_STORE_NESTED_FIELD_OFF(S, NS, F, NF, OFF,    \
> >>>> -                                                      TF);                  \
> >>>> +                     SOCK_ADDR_STORE_NESTED_FIELD_OFF(S, NS, F, NF, SIZE,   \
> >>>> +                                                      OFF, TF);             \
> >>>>                } else {                                                       \
> >>>>                        SOCK_ADDR_LOAD_NESTED_FIELD_SIZE_OFF(                  \
> >>>>                                S, NS, F, NF, SIZE, OFF);  \
> >>>>

^ permalink raw reply

* Re: [PATCH net v2] ipv4: don't set IPv6 only flags to IPv4 addresses
From: David Miller @ 2019-07-01 18:32 UTC (permalink / raw)
  To: mcroce; +Cc: netdev, linux-kernel, kuznet, yoshfuji
In-Reply-To: <20190701170155.1967-1-mcroce@redhat.com>

From: Matteo Croce <mcroce@redhat.com>
Date: Mon,  1 Jul 2019 19:01:55 +0200

> Avoid the situation where an IPV6 only flag is applied to an IPv4 address:
> 
>     # ip addr add 192.0.2.1/24 dev dummy0 nodad home mngtmpaddr noprefixroute
>     # ip -4 addr show dev dummy0
>     2: dummy0: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default qlen 1000
>         inet 192.0.2.1/24 scope global noprefixroute dummy0
>            valid_lft forever preferred_lft forever
> 
> Or worse, by sending a malicious netlink command:
> 
>     # ip -4 addr show dev dummy0
>     2: dummy0: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default qlen 1000
>         inet 192.0.2.1/24 scope global nodad optimistic dadfailed home tentative mngtmpaddr noprefixroute stable-privacy dummy0
>            valid_lft forever preferred_lft forever
> 
> Signed-off-by: Matteo Croce <mcroce@redhat.com>

Applied and queued up for -stable, thanks Matteo.

^ permalink raw reply

* [RESEND PATCH iproute2 net-next] devlink: Introduce PCI PF and VF port flavour and attribute
From: Parav Pandit @ 2019-07-01 18:30 UTC (permalink / raw)
  To: netdev; +Cc: saeedm, jakub.kicinski, jiri, Parav Pandit
In-Reply-To: <20190701122734.18770-1-parav@mellanox.com>

Introduce PCI PF and VF port flavour and port attributes such as PF
number and VF number.

$ devlink port show
pci/0000:05:00.0/0: type eth netdev eth0 flavour pcipf pfnum 0
pci/0000:05:00.0/1: type eth netdev eth1 flavour pcivf pfnum 0 vfnum 0
pci/0000:05:00.0/2: type eth netdev eth2 flavour pcivf pfnum 0 vfnum 1

Acked-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Parav Pandit <parav@mellanox.com>
---
 devlink/devlink.c            | 23 +++++++++++++++++++++++
 include/uapi/linux/devlink.h | 11 +++++++++++
 2 files changed, 34 insertions(+)

diff --git a/devlink/devlink.c b/devlink/devlink.c
index 559f624e..15493426 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -2771,6 +2771,10 @@ static const char *port_flavour_name(uint16_t flavour)
 		return "cpu";
 	case DEVLINK_PORT_FLAVOUR_DSA:
 		return "dsa";
+	case DEVLINK_PORT_FLAVOUR_PCI_PF:
+		return "pcipf";
+	case DEVLINK_PORT_FLAVOUR_PCI_VF:
+		return "pcivf";
 	default:
 		return "<unknown flavour>";
 	}
@@ -2803,8 +2807,27 @@ static void pr_out_port(struct dl *dl, struct nlattr **tb)
 	if (tb[DEVLINK_ATTR_PORT_FLAVOUR]) {
 		uint16_t port_flavour =
 				mnl_attr_get_u16(tb[DEVLINK_ATTR_PORT_FLAVOUR]);
+		uint16_t pf_vf;
 
 		pr_out_str(dl, "flavour", port_flavour_name(port_flavour));
+		if (port_flavour == DEVLINK_PORT_FLAVOUR_PCI_PF) {
+			if (tb[DEVLINK_ATTR_PORT_PCI_PF_NUMBER]) {
+				pf_vf = mnl_attr_get_u16(
+					tb[DEVLINK_ATTR_PORT_PCI_PF_NUMBER]);
+				pr_out_uint(dl, "pfnum", pf_vf);
+			}
+		} else if (port_flavour == DEVLINK_PORT_FLAVOUR_PCI_VF) {
+			if (tb[DEVLINK_ATTR_PORT_PCI_PF_NUMBER]) {
+				pf_vf = mnl_attr_get_u16(
+					tb[DEVLINK_ATTR_PORT_PCI_PF_NUMBER]);
+				pr_out_uint(dl, "pfnum", pf_vf);
+			}
+			if (tb[DEVLINK_ATTR_PORT_PCI_VF_NUMBER]) {
+				pf_vf = mnl_attr_get_u16(
+					tb[DEVLINK_ATTR_PORT_PCI_VF_NUMBER]);
+				pr_out_uint(dl, "vfnum", pf_vf);
+			}
+		}
 	}
 	if (tb[DEVLINK_ATTR_PORT_SPLIT_GROUP])
 		pr_out_uint(dl, "split_group",
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 6544824a..fc195cbd 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -169,6 +169,14 @@ enum devlink_port_flavour {
 	DEVLINK_PORT_FLAVOUR_DSA, /* Distributed switch architecture
 				   * interconnect port.
 				   */
+	DEVLINK_PORT_FLAVOUR_PCI_PF, /* Represents eswitch port for
+				      * the PCI PF. It is an internal
+				      * port that faces the PCI PF.
+				      */
+	DEVLINK_PORT_FLAVOUR_PCI_VF, /* Represents eswitch port
+				      * for the PCI VF. It is an internal
+				      * port that faces the PCI VF.
+				      */
 };
 
 enum devlink_param_cmode {
@@ -337,6 +345,9 @@ enum devlink_attr {
 	DEVLINK_ATTR_FLASH_UPDATE_STATUS_DONE,	/* u64 */
 	DEVLINK_ATTR_FLASH_UPDATE_STATUS_TOTAL,	/* u64 */
 
+	DEVLINK_ATTR_PORT_PCI_PF_NUMBER,	/* u16 */
+	DEVLINK_ATTR_PORT_PCI_VF_NUMBER,	/* u16 */
+
 	/* add new attributes above here, update the policy in devlink.c */
 
 	__DEVLINK_ATTR_MAX,
-- 
2.19.2


^ permalink raw reply related

* Re: [PATCH net-next 1/7] net/rds: Give fr_state a chance to transition to FRMR_IS_FREE
From: David Miller @ 2019-07-01 18:27 UTC (permalink / raw)
  To: gerd.rausch; +Cc: santosh.shilimkar, netdev
In-Reply-To: <44f1794c-7c9c-35bc-dc64-a2a993d06a6e@oracle.com>

From: Gerd Rausch <gerd.rausch@oracle.com>
Date: Mon, 1 Jul 2019 09:39:44 -0700

> +			/* Memory regions make it onto the "clean_list" via
> +			 * "rds_ib_flush_mr_pool", after the memory region has
> +			 * been posted for invalidation via "rds_ib_post_inv".
> +			 *
> +			 * At that point in time, "fr_state" may still be
> +			 * in state "FRMR_IS_INUSE", since the only place where
> +			 * "fr_state" transitions to "FRMR_IS_FREE" is in
> +			 * is in "rds_ib_mr_cqe_handler", which is
> +			 * triggered by a tasklet.
> +			 *
> +			 * So in case we notice that
> +			 * "fr_state != FRMR_IS_FREE" (see below), * we wait for
> +			 * "fr_inv_done" to trigger with a maximum of 10msec.
> +			 * Then we check again, and only put the memory region
> +			 * onto the drop_list (via "rds_ib_free_frmr")
> +			 * in case the situation remains unchanged.
> +			 *
> +			 * This avoids the problem of memory-regions bouncing
> +			 * between "clean_list" and "drop_list" before they
> +			 * even have a chance to be properly invalidated.
> +			 */
> +			frmr = &ibmr->u.frmr;
> +			wait_event_timeout(frmr->fr_inv_done,
> +					   frmr->fr_state == FRMR_IS_FREE,
> +					   msecs_to_jiffies(10));
> +			if (frmr->fr_state == FRMR_IS_FREE)
> +				break;

If we see FRMR_IS_FREE after the timeout, what cleans this up?

Also, why 10msec?  Why that specific value and not some other value?  Why
not wait for however long it takes for the tasklet to run and clean it up?

^ permalink raw reply

* [PATCH] devlink: Introduce PCI PF and VF port flavour and attribute
From: Parav Pandit @ 2019-07-01 18:27 UTC (permalink / raw)
  To: netdev; +Cc: saeedm, jakub.kicinski, jiri, Parav Pandit
In-Reply-To: <20190701122734.18770-1-parav@mellanox.com>

Introduce PCI PF and VF port flavour and port attributes such as PF
number and VF number.

$ devlink port show
pci/0000:05:00.0/0: type eth netdev eth0 flavour pcipf pfnum 0
pci/0000:05:00.0/1: type eth netdev eth1 flavour pcivf pfnum 0 vfnum 0
pci/0000:05:00.0/2: type eth netdev eth2 flavour pcivf pfnum 0 vfnum 1

Acked-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Parav Pandit <parav@mellanox.com>
---
 devlink/devlink.c            | 23 +++++++++++++++++++++++
 include/uapi/linux/devlink.h | 11 +++++++++++
 2 files changed, 34 insertions(+)

diff --git a/devlink/devlink.c b/devlink/devlink.c
index 559f624e..15493426 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -2771,6 +2771,10 @@ static const char *port_flavour_name(uint16_t flavour)
 		return "cpu";
 	case DEVLINK_PORT_FLAVOUR_DSA:
 		return "dsa";
+	case DEVLINK_PORT_FLAVOUR_PCI_PF:
+		return "pcipf";
+	case DEVLINK_PORT_FLAVOUR_PCI_VF:
+		return "pcivf";
 	default:
 		return "<unknown flavour>";
 	}
@@ -2803,8 +2807,27 @@ static void pr_out_port(struct dl *dl, struct nlattr **tb)
 	if (tb[DEVLINK_ATTR_PORT_FLAVOUR]) {
 		uint16_t port_flavour =
 				mnl_attr_get_u16(tb[DEVLINK_ATTR_PORT_FLAVOUR]);
+		uint16_t pf_vf;
 
 		pr_out_str(dl, "flavour", port_flavour_name(port_flavour));
+		if (port_flavour == DEVLINK_PORT_FLAVOUR_PCI_PF) {
+			if (tb[DEVLINK_ATTR_PORT_PCI_PF_NUMBER]) {
+				pf_vf = mnl_attr_get_u16(
+					tb[DEVLINK_ATTR_PORT_PCI_PF_NUMBER]);
+				pr_out_uint(dl, "pfnum", pf_vf);
+			}
+		} else if (port_flavour == DEVLINK_PORT_FLAVOUR_PCI_VF) {
+			if (tb[DEVLINK_ATTR_PORT_PCI_PF_NUMBER]) {
+				pf_vf = mnl_attr_get_u16(
+					tb[DEVLINK_ATTR_PORT_PCI_PF_NUMBER]);
+				pr_out_uint(dl, "pfnum", pf_vf);
+			}
+			if (tb[DEVLINK_ATTR_PORT_PCI_VF_NUMBER]) {
+				pf_vf = mnl_attr_get_u16(
+					tb[DEVLINK_ATTR_PORT_PCI_VF_NUMBER]);
+				pr_out_uint(dl, "vfnum", pf_vf);
+			}
+		}
 	}
 	if (tb[DEVLINK_ATTR_PORT_SPLIT_GROUP])
 		pr_out_uint(dl, "split_group",
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 6544824a..fc195cbd 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -169,6 +169,14 @@ enum devlink_port_flavour {
 	DEVLINK_PORT_FLAVOUR_DSA, /* Distributed switch architecture
 				   * interconnect port.
 				   */
+	DEVLINK_PORT_FLAVOUR_PCI_PF, /* Represents eswitch port for
+				      * the PCI PF. It is an internal
+				      * port that faces the PCI PF.
+				      */
+	DEVLINK_PORT_FLAVOUR_PCI_VF, /* Represents eswitch port
+				      * for the PCI VF. It is an internal
+				      * port that faces the PCI VF.
+				      */
 };
 
 enum devlink_param_cmode {
@@ -337,6 +345,9 @@ enum devlink_attr {
 	DEVLINK_ATTR_FLASH_UPDATE_STATUS_DONE,	/* u64 */
 	DEVLINK_ATTR_FLASH_UPDATE_STATUS_TOTAL,	/* u64 */
 
+	DEVLINK_ATTR_PORT_PCI_PF_NUMBER,	/* u16 */
+	DEVLINK_ATTR_PORT_PCI_VF_NUMBER,	/* u16 */
+
 	/* add new attributes above here, update the policy in devlink.c */
 
 	__DEVLINK_ATTR_MAX,
-- 
2.19.2


^ permalink raw reply related

* Re: [PATCH net] ipv4: don't set IPv6 only flags to IPv4 addresses
From: David Miller @ 2019-07-01 18:22 UTC (permalink / raw)
  To: mcroce; +Cc: joe, netdev, linux-kernel, kuznet, yoshfuji
In-Reply-To: <CAGnkfhx9F1G8K6PjBdUnkCO07GR=ktWAnqOLTcOvg7VGwWb69Q@mail.gmail.com>

From: Matteo Croce <mcroce@redhat.com>
Date: Mon, 1 Jul 2019 18:13:32 +0200

> Can this be edidet on patchwork instead of spamming with a v2?

"Spamming"?

It's never spamming, resends make my life SO much easier.

^ permalink raw reply

* Re: [PATCH net v2] ipv4: don't set IPv6 only flags to IPv4 addresses
From: David Ahern @ 2019-07-01 18:20 UTC (permalink / raw)
  To: Matteo Croce, netdev
  Cc: linux-kernel, David S. Miller, Alexey Kuznetsov,
	Hideaki YOSHIFUJI
In-Reply-To: <c8fac6db-6455-b138-aca9-2f54d782a0b6@gmail.com>

On 7/1/19 11:19 AM, David Ahern wrote:
> I guess at this point we can fail the address add, so this is the best
> option. 

bleh, that should be 'can not'

^ permalink raw reply

* Re: [PATCH v2 net-next 14/19] ionic: Add Tx and Rx handling
From: Shannon Nelson @ 2019-07-01 18:17 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: netdev
In-Reply-To: <20190629115723.5ae777bc@cakuba.netronome.com>

On 6/29/19 11:57 AM, Jakub Kicinski wrote:
> On Fri, 28 Jun 2019 14:39:29 -0700, Shannon Nelson wrote:
>> +static int ionic_tx(struct queue *q, struct sk_buff *skb)
>> +{
>> +	struct tx_stats *stats = q_to_tx_stats(q);
>> +	int err;
>> +
>> +	if (skb->ip_summed == CHECKSUM_PARTIAL)
>> +		err = ionic_tx_calc_csum(q, skb);
>> +	else
>> +		err = ionic_tx_calc_no_csum(q, skb);
>> +	if (err)
>> +		return err;
>> +
>> +	err = ionic_tx_skb_frags(q, skb);
>> +	if (err)
>> +		return err;
>> +
>> +	skb_tx_timestamp(skb);
>> +	stats->pkts++;
>> +	stats->bytes += skb->len;
> Presumably this is 64bit so you should use
> u64_stats_update_begin()
> u64_stats_update_end()
> around it (and all other stats).

Since the device won't work in a 32-bit arch and I have the the Kconfig 
set to depend on 64BIT, I wasn't sure I needed to bother with the extra 
syntactic sugar.

>> +
>> +	ionic_txq_post(q, !netdev_xmit_more(), ionic_tx_clean, skb);
>> +
>> +	return 0;
>> +}
>> +
>> +static int ionic_tx_descs_needed(struct queue *q, struct sk_buff *skb)
>> +{
>> +	struct tx_stats *stats = q_to_tx_stats(q);
>> +	int err;
>> +
>> +	/* If TSO, need roundup(skb->len/mss) descs */
>> +	if (skb_is_gso(skb))
>> +		return (skb->len / skb_shinfo(skb)->gso_size) + 1;
>> +
>> +	/* If non-TSO, just need 1 desc and nr_frags sg elems */
>> +	if (skb_shinfo(skb)->nr_frags <= IONIC_TX_MAX_SG_ELEMS)
>> +		return 1;
>> +
>> +	/* Too many frags, so linearize */
>> +	err = skb_linearize(skb);
>> +	if (err)
>> +		return err;
>> +
>> +	stats->linearize++;
>> +
>> +	/* Need 1 desc and zero sg elems */
>> +	return 1;
>> +}
>> +
>> +netdev_tx_t ionic_start_xmit(struct sk_buff *skb, struct net_device *netdev)
>> +{
>> +	u16 queue_index = skb_get_queue_mapping(skb);
>> +	struct lif *lif = netdev_priv(netdev);
>> +	struct queue *q;
>> +	int ndescs;
>> +	int err;
>> +
>> +	if (unlikely(!test_bit(LIF_UP, lif->state))) {
>> +		dev_kfree_skb(skb);
>> +		return NETDEV_TX_OK;
>> +	}
>> +
>> +	if (likely(lif_to_txqcq(lif, queue_index)))
>> +		q = lif_to_txq(lif, queue_index);
>> +	else
>> +		q = lif_to_txq(lif, 0);
>> +
>> +	ndescs = ionic_tx_descs_needed(q, skb);
>> +	if (ndescs < 0)
>> +		goto err_out_drop;
>> +
>> +	if (!ionic_q_has_space(q, ndescs)) {
>> +		netif_stop_subqueue(netdev, queue_index);
>> +		q->stop++;
>> +
>> +		/* Might race with ionic_tx_clean, check again */
>> +		smp_rmb();
>> +		if (ionic_q_has_space(q, ndescs)) {
>> +			netif_wake_subqueue(netdev, queue_index);
>> +			q->wake++;
>> +		} else {
>> +			return NETDEV_TX_BUSY;
> This should never really happen..

Couldn't we have an Rx interrupt and a call to ionic_tx_clean() in the 
middle of this?

>
>> +		}
>> +	}
>> +
>> +	if (skb_is_gso(skb))
>> +		err = ionic_tx_tso(q, skb);
>> +	else
>> +		err = ionic_tx(q, skb);
>> +
>> +	if (err)
>> +		goto err_out_drop;
> .. at this point if you can't guarantee fitting biggest possible frame
> in, you have to stop the ring.

Yep, that would work.

Thanks,
sln



^ permalink raw reply

* Re: r8169 not working on 5.2.0rc6 with GPD MicroPC
From: Karsten Wiborg @ 2019-07-01 18:15 UTC (permalink / raw)
  To: Andrew Lunn, Heiner Kallweit; +Cc: nic_swsd, romieu, netdev
In-Reply-To: <20190701133507.GB25795@lunn.ch>

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

Hi Andrew, Heiner,

the device is a really small notebook. So detaching mains still leaves
the battery which is delicately built in. So can't currently remove
power completely.

Anyway can I deliver more debugging data to you guys which might add
support for the r8169 for this device?

Regards,
Karsten

On 01/07/2019 15:35, Andrew Lunn wrote:
>> When the vendor driver assigns a random MAC address, it writes it to the
>> chip. The related registers may be persistent (can't say exactly due to
>> missing documentation).
> 
> If the device supports WOL, it could be it is powered using the
> standby supply, not the main supply. Try pulling the plug from the
> wall to really remove all power.
> 
>      Andrew
> 


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4156 bytes --]

^ 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