Netdev List
 help / color / mirror / Atom feed
* Re: [PATCHv2 net-next 1/5] net: dsa: mv88e6xxx: Move phy functions into phy.[ch]
From: David Miller @ 2017-05-25 16:35 UTC (permalink / raw)
  To: andrew; +Cc: vivien.didelot, netdev
In-Reply-To: <1495573486-31827-2-git-send-email-andrew@lunn.ch>

From: Andrew Lunn <andrew@lunn.ch>
Date: Tue, 23 May 2017 23:04:42 +0200

> The upcoming SERDES support will need to make use of PHY functions. Move
> them out into a file of there own. No code changes.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>

Please unexport mv88e6xxx_phy_page_get and mv88e6xxx_phy_page_put, as noted
by Vivien they are not used outside of phy.c

Thank you.

^ permalink raw reply

* Re: Alignment in BPF verifier
From: David Miller @ 2017-05-25 16:31 UTC (permalink / raw)
  To: daniel; +Cc: ast, ecree, alexei.starovoitov, netdev
In-Reply-To: <5924A938.2090808@iogearbox.net>

From: Daniel Borkmann <daniel@iogearbox.net>
Date: Tue, 23 May 2017 23:27:20 +0200

> On 05/23/2017 09:45 PM, Alexei Starovoitov wrote:
>> On 5/23/17 7:41 AM, Edward Cree wrote:
>>> Hmm, that means that we can't do arithmetic on a
>>>  PTR_TO_MAP_VALUE_OR_NULL, we have to convert it to a PTR_TO_MAP_VALUE
>>>  first by NULL-checking it.  That's probably fine, but I can just about
>>>  imagine some compiler optimisation reordering them.  Any reason not to
>>>  split this out into a different reg->field, rather than overloading
>>>  id?
>>
>> 'id' is sort of like 'version' of a pointer and has the same meaning
>> in
>> both cases. How exactly do you see this split?
> 
> Also, same id is never reused once generated and later propagated
> through regs. So far we haven't run into this kind of optimization
> from llvm side yet, but others which led to requiring the id marker
> (see 57a09bf0a416). I could imagine it might be needed at some point,
> though where we later transition directly to PTR_TO_MAP_VALUE_ADJ
> after NULL check. Out of curiosity, did you run into it with llvm?

We could handle this issue in find_good_pkt_pointers(), nothing prevents
us from advancing state there for cases like Edward notes above.

^ permalink raw reply

* Re: [PATCH net-next 1/4] net/flow_dissector: add support for dissection of misc ip header fields
From: Tom Herbert @ 2017-05-25 16:22 UTC (permalink / raw)
  To: Or Gerlitz
  Cc: David S. Miller, Linux Kernel Network Developers, Saeed Mahameed,
	Roi Dayan, Paul Blakey
In-Reply-To: <1495718679-20693-2-git-send-email-ogerlitz@mellanox.com>

On Thu, May 25, 2017 at 6:24 AM, Or Gerlitz <ogerlitz@mellanox.com> wrote:
> Add support for dissection of ip tos and ttl and ipv6 traffic-class
> and hoplimit. Both are dissected into the same struct.
>
> Uses similar call to ip dissection function as with tcp, arp and others.
>
> Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
> Reviewed-by: Jiri Pirko <jiri@mellanox.com>
> ---
>  include/net/flow_dissector.h | 11 +++++++++++
>  net/core/flow_dissector.c    | 40 ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 51 insertions(+)
>
> diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
> index efe34eec..e2663e9 100644
> --- a/include/net/flow_dissector.h
> +++ b/include/net/flow_dissector.h
> @@ -165,6 +165,16 @@ struct flow_dissector_key_tcp {
>         __be16 flags;
>  };
>
> +/**
> + * struct flow_dissector_key_ip:
> + * @tos: tos
> + * @ttl: ttl
> + */
> +struct flow_dissector_key_ip {
> +       __u8    tos;
> +       __u8    ttl;
> +};
> +
>  enum flow_dissector_key_id {
>         FLOW_DISSECTOR_KEY_CONTROL, /* struct flow_dissector_key_control */
>         FLOW_DISSECTOR_KEY_BASIC, /* struct flow_dissector_key_basic */
> @@ -186,6 +196,7 @@ enum flow_dissector_key_id {
>         FLOW_DISSECTOR_KEY_ENC_PORTS, /* struct flow_dissector_key_ports */
>         FLOW_DISSECTOR_KEY_MPLS, /* struct flow_dissector_key_mpls */
>         FLOW_DISSECTOR_KEY_TCP, /* struct flow_dissector_key_tcp */
> +       FLOW_DISSECTOR_KEY_IP, /* struct flow_dissector_key_ip */
>
>         FLOW_DISSECTOR_KEY_MAX,
>  };
> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
> index 5a45943..fc5fc45 100644
> --- a/net/core/flow_dissector.c
> +++ b/net/core/flow_dissector.c
> @@ -367,6 +367,40 @@ __skb_flow_dissect_tcp(const struct sk_buff *skb,
>         key_tcp->flags = (*(__be16 *) &tcp_flag_word(th) & htons(0x0FFF));
>  }
>
> +static void
> +__skb_flow_dissect_ipv4(const struct sk_buff *skb,
> +                       struct flow_dissector *flow_dissector,
> +                       void *target_container, void *data, const struct iphdr *iph)
> +{
> +       struct flow_dissector_key_ip *key_ip;
> +
> +       if (!dissector_uses_key(flow_dissector, FLOW_DISSECTOR_KEY_IP))
> +               return;
> +
> +       key_ip = skb_flow_dissector_target(flow_dissector,
> +                                          FLOW_DISSECTOR_KEY_IP,
> +                                          target_container);
> +       key_ip->tos = iph->tos;
> +       key_ip->ttl = iph->ttl;

In an encapsulation this returns the tos and ttl of the encapsulated
packet. Is that really useful to the caller? Seems more likely that
they need the outer tos and ttl for forwarding.

> +}
> +
> +static void
> +__skb_flow_dissect_ipv6(const struct sk_buff *skb,
> +                       struct flow_dissector *flow_dissector,
> +                       void *target_container, void *data, const struct ipv6hdr *iph)
> +{
> +       struct flow_dissector_key_ip *key_ip;
> +
> +       if (!dissector_uses_key(flow_dissector, FLOW_DISSECTOR_KEY_IP))
> +               return;
> +
> +       key_ip = skb_flow_dissector_target(flow_dissector,
> +                                          FLOW_DISSECTOR_KEY_IP,
> +                                          target_container);
> +       key_ip->tos = ipv6_get_dsfield(iph);
> +       key_ip->ttl = iph->hop_limit;
> +}
> +
>  /**
>   * __skb_flow_dissect - extract the flow_keys struct and return it
>   * @skb: sk_buff to extract the flow from, can be NULL if the rest are specified
> @@ -469,6 +503,9 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
>                         }
>                 }
>
> +               __skb_flow_dissect_ipv4(skb, flow_dissector,
> +                                       target_container, data, iph);
> +
>                 if (flags & FLOW_DISSECTOR_F_STOP_AT_L3)
>                         goto out_good;
>
> @@ -514,6 +551,9 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
>                                 goto out_good;
>                 }
>
> +               __skb_flow_dissect_ipv6(skb, flow_dissector,
> +                                       target_container, data, iph);
> +
>                 if (flags & FLOW_DISSECTOR_F_STOP_AT_L3)
>                         goto out_good;
>
> --
> 2.3.7
>

^ permalink raw reply

* [PATCH net-next v2 3/3] net: sysfs: Document PHY device sysfs attributes
From: Florian Fainelli @ 2017-05-25 16:21 UTC (permalink / raw)
  To: netdev; +Cc: davem, andrew, Florian Fainelli
In-Reply-To: <20170525162143.18319-1-f.fainelli@gmail.com>

Document the different sysfs attributes that exist for PHY devices:
attached_dev, phy_has_fixups, phy_id and phy_interface.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 Documentation/ABI/testing/sysfs-class-net-phydev | 36 ++++++++++++++++++++++++
 1 file changed, 36 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-class-net-phydev

diff --git a/Documentation/ABI/testing/sysfs-class-net-phydev b/Documentation/ABI/testing/sysfs-class-net-phydev
new file mode 100644
index 000000000000..c768d5fd8496
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-net-phydev
@@ -0,0 +1,36 @@
+What:		/sys/class/mdio_bus/<bus>/<device>/attached_dev
+Date:		May 2017
+KernelVersion:	4.13
+Contact:	netdev@vger.kernel.org
+Description:
+		Symbolic link to the network device this PHY device is
+		attached to.
+
+What:		/sys/class/mdio_bus/<bus>/<device>/phy_has_fixups
+Date:		February 2014
+KernelVersion:	3.15
+Contact:	netdev@vger.kernel.org
+Description:
+		Boolean value indicating whether the PHY device has
+		any fixups registered against it (phy_register_fixup)
+
+What:		/sys/class/mdio_bus/<bus>/<device>/phy_id
+Date:		November 2012
+KernelVersion:	3.8
+Contact:	netdev@vger.kernel.org
+Description:
+		32-bit hexadecimal value corresponding to the PHY device's OUI,
+		model and revision number.
+
+What:		/sys/class/mdio_bus/<bus>/<device>/phy_interface
+Date:		February 2014
+KernelVersion:	3.15
+Contact:	netdev@vger.kernel.org
+Description:
+		String value indicating the PHY interface, possible
+		values are:.
+		<empty> (not available), mii, gmii, sgmii, tbi, rev-mii,
+		rmii, rgmii, rgmii-id, rgmii-rxid, rgmii-txid, rtbi, smii
+		xgmii, moca, qsgmii, trgmii, 1000base-x, 2500base-x, rxaui,
+		unknown
+
-- 
2.9.3

^ permalink raw reply related

* [PATCH net-next v2 2/3] net: sysfs: Document "phydev" symbolic link
From: Florian Fainelli @ 2017-05-25 16:21 UTC (permalink / raw)
  To: netdev; +Cc: davem, andrew, Florian Fainelli
In-Reply-To: <20170525162143.18319-1-f.fainelli@gmail.com>

Now that we link the network device to its PHY device, document this
sysfs symbolic link.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 Documentation/ABI/testing/sysfs-class-net | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-class-net b/Documentation/ABI/testing/sysfs-class-net
index 668604fc8e06..6856da99b6f7 100644
--- a/Documentation/ABI/testing/sysfs-class-net
+++ b/Documentation/ABI/testing/sysfs-class-net
@@ -251,3 +251,11 @@ Contact:	netdev@vger.kernel.org
 Description:
 		Indicates the unique physical switch identifier of a switch this
 		port belongs to, as a string.
+
+What:		/sys/class/net/<iface>/phydev
+Date:		May 2017
+KernelVersion:	4.13
+Contact:	netdev@vger.kernel.org
+Description:
+		Symbolic link to the PHY device this network device is attached
+		to.
-- 
2.9.3

^ permalink raw reply related

* [PATCH net-next v2 1/3] net: phy: Create sysfs reciprocal links for attached_dev/phydev
From: Florian Fainelli @ 2017-05-25 16:21 UTC (permalink / raw)
  To: netdev; +Cc: davem, andrew, Florian Fainelli
In-Reply-To: <20170525162143.18319-1-f.fainelli@gmail.com>

There is currently no way for a program scanning /sys to know whether a
network device is attached to a particular PHY device, just like the PHY
device is not pointed back to its attached network device.

Create a symbolic link in the network device's namespace named "phydev"
which points to the PHY device and create a symbolic link in the PHY
device's namespace named "attached_dev" that points back to the network
device. These links are set up during phy_attach_direct() and removed
during phy_detach() for symetry.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/phy/phy_device.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 1219eeab69d1..8151477c7027 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -960,6 +960,15 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
 
 	phydev->attached_dev = dev;
 	dev->phydev = phydev;
+	err = sysfs_create_link(&phydev->mdio.dev.kobj, &dev->dev.kobj,
+				"attached_dev");
+	if (err)
+		goto error;
+
+	err = sysfs_create_link(&dev->dev.kobj, &phydev->mdio.dev.kobj,
+				"phydev");
+	if (err)
+		goto error;
 
 	phydev->dev_flags = flags;
 
@@ -1050,6 +1059,8 @@ void phy_detach(struct phy_device *phydev)
 	struct mii_bus *bus;
 	int i;
 
+	sysfs_remove_link(&dev->dev.kobj, "phydev");
+	sysfs_remove_link(&phydev->mdio.dev.kobj, "attached_dev");
 	phydev->attached_dev->phydev = NULL;
 	phydev->attached_dev = NULL;
 	phy_suspend(phydev);
-- 
2.9.3

^ permalink raw reply related

* [PATCH net-next v2 0/3] net: phy: Create sysfs reciprocal links for attached_dev/phydev
From: Florian Fainelli @ 2017-05-25 16:21 UTC (permalink / raw)
  To: netdev; +Cc: davem, andrew, Florian Fainelli

Hi David, Andrew,

This patch series addresses a device topology shortcoming where a program
scanning /sys would not be able to establish a mapping between the network
device and the PHY device.

In the process it turned out that no PHY device documentation existed for
sysfs attributes.

Thanks!

Changes in v2:

- document possible phy_interface values in sysfs-class-net-phydev

Florian Fainelli (3):
  net: phy: Create sysfs reciprocal links for attached_dev/phydev
  net: sysfs: Document "phydev" symbolic link
  net: sysfs: Document PHY device sysfs attributes

 Documentation/ABI/testing/sysfs-class-net        |  8 ++++++
 Documentation/ABI/testing/sysfs-class-net-phydev | 36 ++++++++++++++++++++++++
 drivers/net/phy/phy_device.c                     | 11 ++++++++
 3 files changed, 55 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-class-net-phydev

-- 
2.9.3

^ permalink raw reply

* Re: [Patch net-next] net_sched: only create filter chains for new filters/actions
From: David Miller @ 2017-05-25 16:14 UTC (permalink / raw)
  To: xiyou.wangcong; +Cc: netdev, jhs, jiri
In-Reply-To: <1495557757-31371-1-git-send-email-xiyou.wangcong@gmail.com>

From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Tue, 23 May 2017 09:42:37 -0700

> tcf_chain_get() always creates a new filter chain if not found
> in existing ones. This is totally unnecessary when we get or
> delete filters, new chain should be only created for new filters
> (or new actions).
> 
> Fixes: 5bc1701881e3 ("net: sched: introduce multichain support for filters")
> Cc: Jamal Hadi Salim <jhs@mojatatu.com>
> Cc: Jiri Pirko <jiri@mellanox.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>

Indeed, get and delete requests should not create new objects, ever.

I have pretty much no idea why Jiri is making such a big fuss about
this change, to be quite honest. :-)

Applied, thanks Cong.

^ permalink raw reply

* Re: [patch net-next] net: sched: cls_api: make reclassify return all the way back to the original tp
From: David Miller @ 2017-05-25 16:12 UTC (permalink / raw)
  To: jiri
  Cc: netdev, jhs, xiyou.wangcong, dsa, edumazet, daniel,
	alexander.h.duyck, simon.horman, mlxsw
In-Reply-To: <20170523071159.1951-1-jiri@resnulli.us>

From: Jiri Pirko <jiri@resnulli.us>
Date: Tue, 23 May 2017 09:11:59 +0200

> From: Jiri Pirko <jiri@mellanox.com>
> 
> With the introduction of chain goto action, the reclassification would
> cause the re-iteration of the actual chain. It makes more sense to restart
> the whole thing and re-iterate starting from the original tp - start
> of chain 0.
> 
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>

Applied, thanks Jiri.

^ permalink raw reply

* Re: [PATCH net] ip6_tunnel, ip6_gre: fix setting of DSCP on encapsulated packets
From: David Miller @ 2017-05-25 16:11 UTC (permalink / raw)
  To: petedaws; +Cc: kuznet, jmorris, yoshfuji, kaber, stephen, netdev, linux-kernel
In-Reply-To: <20170523143616.38116e8f@gmail.com>

From: Peter Dawson <petedaws@gmail.com>
Date: Tue, 23 May 2017 14:36:16 +1000

> Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=195661

This is not the correct way to use the Fixes: tag.

You should specify the commit that introduced the regression
between 4.9.x and 4.10.x, and that you are fixing here.

The correct format for the commit references is the 12 initial digits
of the SHA1_ID of the commit, followed by a space, followed by the
commit log message header line text enclosed in parentheses and
double quotes with no line breaks whatsoever.

Thank you.

^ permalink raw reply

* Re: [PATCH net-next] bpf: update perf event helper function signature and documentation
From: David Miller @ 2017-05-25 16:07 UTC (permalink / raw)
  To: qinteng; +Cc: peterz, bgregg, daniel, netdev, linux-kernel, Kernel-team, ast
In-Reply-To: <BB53B290-EED3-4B92-8D2D-33954D7B6C68@fb.com>

From: Teng Qin <qinteng@fb.com>
Date: Tue, 23 May 2017 03:17:31 +0000

> Alexei said it was due to Clang not taking u64, u32 etc. for compilation.
> I didn’t know the context and just used them. But apparently, something
> changed and now they build and run OK......

These types are %100 compiler agnostic.  Clang doesn't care.  All
compilers can understand:

	typedef unsigned int u32;

So it's not a Clang specific issue.

The problem could only have to do with whether the types get typdef'd
or not with the header files that are used in conjunction with the
file we are talking about here.

And if we do get a proper set of defines from linux/types.h these days
we should:

1) Make that explicit, by including linux/types.h in bpf_helpers.h

2) Use those types universally in this file where we were avoiding
   doing so

This header file also uses "SEC" and expects to get this definition
implicitly, and therefore there is another header file that
bpf_helpers.h must include in order to explicitly and reliably have
that definition.

Just changing one prototype, and relying on getting the types
indirectly by other header files including by whoever incudes
bpf_helpers.h is not the way to go.

Thanks.


^ permalink raw reply

* Re: [pull request][for-next 0/6] Mellanox mlx5 updates 2017-05-23
From: David Miller @ 2017-05-25 16:02 UTC (permalink / raw)
  To: saeedm-VPRAkNaXOzVWk0Htik3J/w
  Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA, ilant-VPRAkNaXOzVWk0Htik3J/w
In-Reply-To: <20170523114404.20387-1-saeedm-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

From: Saeed Mahameed <saeedm-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Date: Tue, 23 May 2017 14:43:58 +0300

> Hi Dave and Doug,
> 
> This series introduces some small updates and FPGA support to the mlx5
> core/ethernet and IB drivers.
> 
> For more details please see below.
> 
> Please pull and let me know if there's any problem.

Ok, I've pulled this into net-next.

Doug let me know if there are any merge hassles we need to coordinate on.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH] i40e: Fix incorrect pf->flags
From: tndave @ 2017-05-25 16:00 UTC (permalink / raw)
  To: Jeff Kirsher, intel-wired-lan, netdev
In-Reply-To: <1495703624.49259.39.camel@intel.com>



On 05/25/2017 02:13 AM, Jeff Kirsher wrote:
> On Fri, 2017-05-19 at 18:01 -0700, Tushar Dave wrote:
>> Fix bug introduced by 'commit 47994c119a36e ("i40e: remove
>> hw_disabled_flags in favor of using separate flag bits")' that
>> mistakenly wipes out pf->flags.
>>
>> Signed-off-by: Tushar Dave <tushar.n.dave@oracle.com>
>> ---
>>  drivers/net/ethernet/intel/i40e/i40e_main.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> :-(
> Not even close to applying, I even tried to apply by hand but the patch
> was not near the same as my branch in my tree.  I will forgive this
> second instance of a patch generated on different tree/branch, but I
> really must insist that you use my tree & branch if you need/want to
> make changes to Intel wired LAN driver changes.
Apology Jeff. I used Dave's net tree.
Now onwards, I will use your 'net-queue:dev_queue' for bug fixes and
'next-queue:dev_queue' for developmental changes to Intel Ethernet drivers.

Being this patch a bug fix, I will send v2 based off your git tree
'net-queue' on branch 'dev_queue'. Hope that is okay?

Thanks.

-Tushar
>
>>
>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
>> b/drivers/net/ethernet/intel/i40e/i40e_main.c
>> index d5c9c9e..6b98d34 100644
>> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
>> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
>> @@ -8821,9 +8821,9 @@ static int i40e_sw_init(struct i40e_pf *pf)
>>  		    (pf->hw.aq.api_min_ver > 4))) {
>>  		/* Supported in FW API version higher than 1.4 */
>>  		pf->flags |= I40E_FLAG_GENEVE_OFFLOAD_CAPABLE;
>> -		pf->flags = I40E_FLAG_HW_ATR_EVICT_CAPABLE;
>> +		pf->flags |= I40E_FLAG_HW_ATR_EVICT_CAPABLE;
>>  	} else {
>> -		pf->flags = I40E_FLAG_HW_ATR_EVICT_CAPABLE;
>> +		pf->flags |= I40E_FLAG_HW_ATR_EVICT_CAPABLE;
>>  	}
>>
>>  	pf->eeprom_version = 0xDEAD;

^ permalink raw reply

* Re: [PATCH net-next 8/8] net: ipv6: RTM_GETROUTE: return matched fib result when requested
From: David Ahern @ 2017-05-25 16:00 UTC (permalink / raw)
  To: Roopa Prabhu
  Cc: davem@davemloft.net, netdev@vger.kernel.org, Nikolay Aleksandrov
In-Reply-To: <CAJieiUh+GAtOGZv4UU_gcn5isA6DSqaFj_EXzA24qBK7vJz4AQ@mail.gmail.com>


> On May 25, 2017, at 9:54 AM, Roopa Prabhu <roopa@cumulusnetworks.com> wrote:
> 
>> 
>> I'd prefer to see the typecasts go away and use container_of to go from
>> dst_entry to rt6_info. I realize some of this is movement of existing
>> code, but better to clean up as we go.
>> 
> 
> ack. But that is pretty much all of ipv6 code. so, seems better done
> with an incremental cleanup patch.
> 

For the rest of the ipv6 code, yes. We can do the right thing now for new code and changes to existing code.

^ permalink raw reply

* Re: [PATCH 0/2] Replace driver's usage of hard-coded device IDs to #defines
From: Myron Stowe @ 2017-05-25 15:56 UTC (permalink / raw)
  To: David Miller
  Cc: myron.stowe, linux-pci, netdev, bhelgaas, saeedm, noaos, tariqt
In-Reply-To: <20170524.200249.1492095559302424098.davem@davemloft.net>

On Wed, 24 May 2017 20:02:49 -0400 (EDT)
David Miller <davem@davemloft.net> wrote:

> From: Myron Stowe <myron.stowe@redhat.com>
> Date: Wed, 24 May 2017 16:47:34 -0600
> 
> > Noa Osherovich introduced a series of new Mellanox device ID
> > definitions to help differentiate specific controllers that needed
> > INTx masking quirks [1].
> > 
> > Bjorn Helgaas followed on, using the device ID definitions Noa
> > provided to replace hard-coded values within the mxl4 ID table [2].
> >     
> > This patch continues along similar lines, adding a few additional
> > Mellanox device ID definitions and converting the net/mlx5e
> > driver's mlx5 ID table to use the defines so tools like 'grep' and
> > 'cscope' can be used to help identify relationships with other
> > aspects (such as INTx masking).  
> 
> If you're adding pci_ids.h defines, it's only valid to do so if you
> actually use the defines in more than one location.
> 
> This patch series is not doing that.

Hi David,

Yes, now that you mention that again I do vaguely remember past
conversations stating similar constraints which is a little odd as
Noa's series did exactly that.  It was Bjorn, in a separate patch, that
made the connection to the driver with commit c19e4b9037f
("net/mlx4_core: Use device ID defines") [1] and even after such, some
of the introduced #defines are still currently singular in usage.

Anyway, the part I'm interested in is creating a more transparent
association between the Mellanox controllers that need the INTx masking
quirk and their drivers, something that remains very opaque currently
for a few of the remaining instances (PCI_DEVICE_ID_MELLANOX_CONNECTIB,
PCI_DEVICE_ID_MELLANOX_CONNECTX4, and
PCI_DEVICE_ID_MELLANOX_CONNECTX4_LX).

I'd like to hear back from others as to whether or not there is truly
concern about adding the #defines such as I submitted with singular
usages and if so I can re-submit a more focused patch which would
effectively be the first three substitutions in "[PATCH 2/2] net/mlx5e:
Use device ID defines".


[1] Perhaps Noa's submission had a similar discussion as while it was
a separate series from which Bjorn then made the connection to the
driver, all the patches came in via the same merge commit 25831571419
("Merge branch 'pci/virtualization' into next").


 Thanks for your
feedback,
 Myron

^ permalink raw reply

* Re: [PATCH net-next 8/8] net: ipv6: RTM_GETROUTE: return matched fib result when requested
From: Roopa Prabhu @ 2017-05-25 15:54 UTC (permalink / raw)
  To: David Ahern
  Cc: davem@davemloft.net, netdev@vger.kernel.org, Nikolay Aleksandrov
In-Reply-To: <2106987d-7c19-b739-0103-97af3a3071da@gmail.com>

On Wed, May 24, 2017 at 7:35 PM, David Ahern <dsahern@gmail.com> wrote:
> Since you have to do a v2 ...
>
> On 5/24/17 12:19 PM, Roopa Prabhu wrote:
>> @@ -3622,6 +3623,7 @@ static int inet6_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh,
>>       memset(&fl6, 0, sizeof(fl6));
>>       rtm = nlmsg_data(nlh);
>>       fl6.flowlabel = ip6_make_flowinfo(rtm->rtm_tos, 0);
>> +     fibmatch = (rtm->rtm_flags & RTM_F_FIB_MATCH) ? true : false;
> this is typically done as !!(rtm->rtm_flags & RTM_F_FIB_MATCH)

ack,


>>
>>       if (tb[RTA_SRC]) {
>>               if (nla_len(tb[RTA_SRC]) < sizeof(struct in6_addr))
>> @@ -3667,12 +3669,27 @@ static int inet6_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh,
>>               if (!ipv6_addr_any(&fl6.saddr))
>>                       flags |= RT6_LOOKUP_F_HAS_SADDR;
>>
>> -             rt = (struct rt6_info *)ip6_route_input_lookup(net, dev, &fl6,
>> -                                                            flags);
>> +             if (!fibmatch)
>> +                     rt = (struct rt6_info *)ip6_route_input_lookup(net, dev,
>> +                                                                    &fl6,
>> +                                                                    flags);
>>       } else {
>>               fl6.flowi6_oif = oif;
>>
>> -             rt = (struct rt6_info *)ip6_route_output(net, NULL, &fl6);
>> +             if (!fibmatch)
>> +                     rt = (struct rt6_info *)ip6_route_output_flags(net,
>> +                                                                    NULL,
>> +                                                                    &fl6, 0);
>> +     }
>> +
>> +     if (fibmatch) {
>> +             rt = (struct rt6_info *)ip6_route_lookup(net, &fl6, 0);
>> +             if (rt->dst.error) {
>> +                     err = rt->dst.error;
>> +                     ip6_rt_put(rt);
>> +                     goto errout;
>> +             }
>> +
>
> I'd prefer to see the typecasts go away and use container_of to go from
> dst_entry to rt6_info. I realize some of this is movement of existing
> code, but better to clean up as we go.
>

ack. But that is pretty much all of ipv6 code. so, seems better done
with an incremental cleanup patch.


thanks for the review.

^ permalink raw reply

* Re: [PATCH net-next 1/4] net/flow_dissector: add support for dissection of misc ip header fields
From: Tom Herbert @ 2017-05-25 15:42 UTC (permalink / raw)
  To: Or Gerlitz
  Cc: David S. Miller, Linux Kernel Network Developers, Saeed Mahameed,
	Roi Dayan, Paul Blakey
In-Reply-To: <1495718679-20693-2-git-send-email-ogerlitz@mellanox.com>

On Thu, May 25, 2017 at 6:24 AM, Or Gerlitz <ogerlitz@mellanox.com> wrote:
> Add support for dissection of ip tos and ttl and ipv6 traffic-class
> and hoplimit. Both are dissected into the same struct.
>
> Uses similar call to ip dissection function as with tcp, arp and others.
>
> Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
> Reviewed-by: Jiri Pirko <jiri@mellanox.com>
> ---
>  include/net/flow_dissector.h | 11 +++++++++++
>  net/core/flow_dissector.c    | 40 ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 51 insertions(+)
>
> diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
> index efe34eec..e2663e9 100644
> --- a/include/net/flow_dissector.h
> +++ b/include/net/flow_dissector.h
> @@ -165,6 +165,16 @@ struct flow_dissector_key_tcp {
>         __be16 flags;
>  };
>
> +/**
> + * struct flow_dissector_key_ip:
> + * @tos: tos
> + * @ttl: ttl
> + */
> +struct flow_dissector_key_ip {
> +       __u8    tos;
> +       __u8    ttl;
> +};
> +

Looks like yet more complexity be piled onto flow dissector. Instead
of splitting out individual fields can we just return a pointer to the
IP header and let the caller extract the fields they're interested in?

Tom

>  enum flow_dissector_key_id {
>         FLOW_DISSECTOR_KEY_CONTROL, /* struct flow_dissector_key_control */
>         FLOW_DISSECTOR_KEY_BASIC, /* struct flow_dissector_key_basic */
> @@ -186,6 +196,7 @@ enum flow_dissector_key_id {
>         FLOW_DISSECTOR_KEY_ENC_PORTS, /* struct flow_dissector_key_ports */
>         FLOW_DISSECTOR_KEY_MPLS, /* struct flow_dissector_key_mpls */
>         FLOW_DISSECTOR_KEY_TCP, /* struct flow_dissector_key_tcp */
> +       FLOW_DISSECTOR_KEY_IP, /* struct flow_dissector_key_ip */
>
>         FLOW_DISSECTOR_KEY_MAX,
>  };
> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
> index 5a45943..fc5fc45 100644
> --- a/net/core/flow_dissector.c
> +++ b/net/core/flow_dissector.c
> @@ -367,6 +367,40 @@ __skb_flow_dissect_tcp(const struct sk_buff *skb,
>         key_tcp->flags = (*(__be16 *) &tcp_flag_word(th) & htons(0x0FFF));
>  }
>
> +static void
> +__skb_flow_dissect_ipv4(const struct sk_buff *skb,
> +                       struct flow_dissector *flow_dissector,
> +                       void *target_container, void *data, const struct iphdr *iph)
> +{
> +       struct flow_dissector_key_ip *key_ip;
> +
> +       if (!dissector_uses_key(flow_dissector, FLOW_DISSECTOR_KEY_IP))
> +               return;
> +
> +       key_ip = skb_flow_dissector_target(flow_dissector,
> +                                          FLOW_DISSECTOR_KEY_IP,
> +                                          target_container);
> +       key_ip->tos = iph->tos;
> +       key_ip->ttl = iph->ttl;
> +}
> +
> +static void
> +__skb_flow_dissect_ipv6(const struct sk_buff *skb,
> +                       struct flow_dissector *flow_dissector,
> +                       void *target_container, void *data, const struct ipv6hdr *iph)
> +{
> +       struct flow_dissector_key_ip *key_ip;
> +
> +       if (!dissector_uses_key(flow_dissector, FLOW_DISSECTOR_KEY_IP))
> +               return;
> +
> +       key_ip = skb_flow_dissector_target(flow_dissector,
> +                                          FLOW_DISSECTOR_KEY_IP,
> +                                          target_container);
> +       key_ip->tos = ipv6_get_dsfield(iph);
> +       key_ip->ttl = iph->hop_limit;
> +}
> +
>  /**
>   * __skb_flow_dissect - extract the flow_keys struct and return it
>   * @skb: sk_buff to extract the flow from, can be NULL if the rest are specified
> @@ -469,6 +503,9 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
>                         }
>                 }
>
> +               __skb_flow_dissect_ipv4(skb, flow_dissector,
> +                                       target_container, data, iph);
> +
>                 if (flags & FLOW_DISSECTOR_F_STOP_AT_L3)
>                         goto out_good;
>
> @@ -514,6 +551,9 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
>                                 goto out_good;
>                 }
>
> +               __skb_flow_dissect_ipv6(skb, flow_dissector,
> +                                       target_container, data, iph);
> +
>                 if (flags & FLOW_DISSECTOR_F_STOP_AT_L3)
>                         goto out_good;
>
> --
> 2.3.7
>

^ permalink raw reply

* [PATCH V5 iproute] ip: Add support for netdev events to monitor
From: Vladislav Yasevich @ 2017-05-25 15:31 UTC (permalink / raw)
  To: netdev; +Cc: dsahern, roopa, jiri, vfalico, andy, Vladislav Yasevich
In-Reply-To: <1495726316-27626-1-git-send-email-vyasevic@redhat.com>

Add IFLA_EVENT handling so that event types can be viewed with
'monitor' command.  This gives a little more information for why
a given message was receivied.

Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
---
 include/linux/if_link.h | 11 +++++++++++
 ip/ipaddress.c          | 21 +++++++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/include/linux/if_link.h b/include/linux/if_link.h
index 5a3a048..7e10274 100644
--- a/include/linux/if_link.h
+++ b/include/linux/if_link.h
@@ -157,6 +157,7 @@ enum {
 	IFLA_GSO_MAX_SIZE,
 	IFLA_PAD,
 	IFLA_XDP,
+	IFLA_EVENT,
 	__IFLA_MAX
 };
 
@@ -909,4 +910,14 @@ enum {
 
 #define IFLA_XDP_MAX (__IFLA_XDP_MAX - 1)
 
+enum {
+	IFLA_EVENT_UNSPEC,
+	IFLA_EVENT_REBOOT,
+	IFLA_EVENT_FEAT_CHANGE,
+	IFLA_EVENT_BONDING_FAILOVER,
+	IFLA_EVENT_NOTIFY_PEERS,
+	IFLA_EVENT_RESEND_IGMP,
+	IFLA_EVENT_CHANGE_INFO_DATA,
+};
+
 #endif /* _LINUX_IF_LINK_H */
diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index b8d9c7d..d37b4b2 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -753,6 +753,24 @@ int print_linkinfo_brief(const struct sockaddr_nl *who,
 	return 0;
 }
 
+static const char *netdev_events[] = {"UNKNOWN",
+				      "REBOOT",
+				      "FEATURE_CHANGE",
+				      "BONDING_FAILOVER",
+				      "NOTIFY_PEERS",
+				      "RESEND_IGMP",
+				      "CHANGE_INFO_DATA"};
+
+static void print_dev_event(FILE *f, __u32 event)
+{
+	if (event >= ARRAY_SIZE(netdev_events))
+		fprintf(f, "event %d ", event);
+	else {
+		if (event)
+			fprintf(f, "event %s ", netdev_events[event]);
+	}
+}
+
 int print_linkinfo(const struct sockaddr_nl *who,
 		   struct nlmsghdr *n, void *arg)
 {
@@ -858,6 +876,9 @@ int print_linkinfo(const struct sockaddr_nl *who,
 	if (filter.showqueue)
 		print_queuelen(fp, tb);
 
+	if (tb[IFLA_EVENT])
+		print_dev_event(fp, rta_getattr_u32(tb[IFLA_EVENT]));
+
 	if (!filter.family || filter.family == AF_PACKET || show_details) {
 		SPRINT_BUF(b1);
 		fprintf(fp, "%s", _SL_);
-- 
2.7.4

^ permalink raw reply related

* [PATCH V5 2/2] bonding: Prevent duplicate userspace notification
From: Vladislav Yasevich @ 2017-05-25 15:31 UTC (permalink / raw)
  To: netdev; +Cc: dsahern, roopa, jiri, vfalico, andy, Vladislav Yasevich
In-Reply-To: <1495726316-27626-1-git-send-email-vyasevic@redhat.com>

Whenever a user changes bonding options, a NETDEV_CHANGEINFODATA
notificatin is generated which results in a rtnelink message to
be sent.  While runnig 'ip monitor', we can actually see 2 messages,
one a result of the event, and the other a result of state change
that is generated bo netdev_state_change().  However, this is not
always the case. If bonding changes were done via sysfs or ifenslave
(old ioctl interface), then only 1 message is seen.

This patch removes duplicate messages in the case of using netlink
to configure bonding.  It introduceds a separte function that
triggers a netdev event and uses that function in the syfs and ioctl
cases.

This was discovered while auditing all the different envents and
continues the effort of cleaning up duplicated netlink messages.

Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
---
 drivers/net/bonding/bond_main.c    |  3 ++-
 drivers/net/bonding/bond_options.c | 27 +++++++++++++++++++++++++--
 include/net/bond_options.h         |  2 ++
 3 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 7331331..d7aa137 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3481,7 +3481,8 @@ static int bond_do_ioctl(struct net_device *bond_dev, struct ifreq *ifr, int cmd
 	case BOND_CHANGE_ACTIVE_OLD:
 	case SIOCBONDCHANGEACTIVE:
 		bond_opt_initstr(&newval, slave_dev->name);
-		res = __bond_opt_set(bond, BOND_OPT_ACTIVE_SLAVE, &newval);
+		res = __bond_opt_set_notify(bond, BOND_OPT_ACTIVE_SLAVE,
+					    &newval);
 		break;
 	default:
 		res = -EOPNOTSUPP;
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index 1bcbb89..8ca6833 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -673,7 +673,30 @@ int __bond_opt_set(struct bonding *bond,
 out:
 	if (ret)
 		bond_opt_error_interpret(bond, opt, ret, val);
-	else if (bond->dev->reg_state == NETREG_REGISTERED)
+
+	return ret;
+}
+/**
+ * __bond_opt_set_notify - set a bonding option
+ * @bond: target bond device
+ * @option: option to set
+ * @val: value to set it to
+ *
+ * This function is used to change the bond's option value and trigger
+ * a notification to user sapce. It can be used for both enabling/changing
+ * an option and for disabling it. RTNL lock must be obtained before calling
+ * this function.
+ */
+int __bond_opt_set_notify(struct bonding *bond,
+			  unsigned int option, struct bond_opt_value *val)
+{
+	int ret = -ENOENT;
+
+	ASSERT_RTNL();
+
+	ret = __bond_opt_set(bond, option, val);
+
+	if (!ret && (bond->dev->reg_state == NETREG_REGISTERED))
 		call_netdevice_notifiers(NETDEV_CHANGEINFODATA, bond->dev);
 
 	return ret;
@@ -696,7 +719,7 @@ int bond_opt_tryset_rtnl(struct bonding *bond, unsigned int option, char *buf)
 	if (!rtnl_trylock())
 		return restart_syscall();
 	bond_opt_initstr(&optval, buf);
-	ret = __bond_opt_set(bond, option, &optval);
+	ret = __bond_opt_set_notify(bond, option, &optval);
 	rtnl_unlock();
 
 	return ret;
diff --git a/include/net/bond_options.h b/include/net/bond_options.h
index 1797235..d79d28f 100644
--- a/include/net/bond_options.h
+++ b/include/net/bond_options.h
@@ -104,6 +104,8 @@ struct bond_option {
 
 int __bond_opt_set(struct bonding *bond, unsigned int option,
 		   struct bond_opt_value *val);
+int __bond_opt_set_notify(struct bonding *bond, unsigned int option,
+			  struct bond_opt_value *val);
 int bond_opt_tryset_rtnl(struct bonding *bond, unsigned int option, char *buf);
 
 const struct bond_opt_value *bond_opt_parse(const struct bond_option *opt,
-- 
2.7.4

^ permalink raw reply related

* [PATCH V5 1/2] rtnl: Add support for netdev event to link messages
From: Vladislav Yasevich @ 2017-05-25 15:31 UTC (permalink / raw)
  To: netdev; +Cc: dsahern, roopa, jiri, vfalico, andy, Vladislav Yasevich
In-Reply-To: <1495726316-27626-1-git-send-email-vyasevic@redhat.com>

When netdev events happen, a rtnetlink_event() handler will send
messages for every event in it's white list.  These messages contain
current information about a particular device, but they do not include
the iformation about which event just happened.  So, it is impossible
to tell what just happend for these events.

This patch adds a new extension to RTM_NEWLINK message called IFLA_EVENT
that would have an encoding of event that triggered this
message.  This would allow the the message consumer to easily determine
if it needs to perform certain actions.

Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
---
 include/linux/rtnetlink.h    |  3 ++-
 include/uapi/linux/if_link.h | 11 ++++++++
 net/core/dev.c               |  2 +-
 net/core/rtnetlink.c         | 62 +++++++++++++++++++++++++++++++++++++-------
 4 files changed, 67 insertions(+), 11 deletions(-)

diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
index 57e5484..0459018 100644
--- a/include/linux/rtnetlink.h
+++ b/include/linux/rtnetlink.h
@@ -18,7 +18,8 @@ extern int rtnl_put_cacheinfo(struct sk_buff *skb, struct dst_entry *dst,
 
 void rtmsg_ifinfo(int type, struct net_device *dev, unsigned change, gfp_t flags);
 struct sk_buff *rtmsg_ifinfo_build_skb(int type, struct net_device *dev,
-				       unsigned change, gfp_t flags);
+				       unsigned change, unsigned long event,
+				       gfp_t flags);
 void rtmsg_ifinfo_send(struct sk_buff *skb, struct net_device *dev,
 		       gfp_t flags);
 
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 15ac203..8779ab7 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -157,6 +157,7 @@ enum {
 	IFLA_GSO_MAX_SIZE,
 	IFLA_PAD,
 	IFLA_XDP,
+	IFLA_EVENT,
 	__IFLA_MAX
 };
 
@@ -911,4 +912,14 @@ enum {
 
 #define IFLA_XDP_MAX (__IFLA_XDP_MAX - 1)
 
+enum {
+	IFLA_EVENT_UNSPEC,
+	IFLA_EVENT_REBOOT,
+	IFLA_EVENT_FEAT_CHANGE,
+	IFLA_EVENT_BONDING_FAILOVER,
+	IFLA_EVENT_NOTIFY_PEERS,
+	IFLA_EVENT_RESEND_IGMP,
+	IFLA_EVENT_CHANGE_INFO_DATA,
+};
+
 #endif /* _UAPI_LINUX_IF_LINK_H */
diff --git a/net/core/dev.c b/net/core/dev.c
index 3d98fbf..06e0a74 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -7084,7 +7084,7 @@ static void rollback_registered_many(struct list_head *head)
 
 		if (!dev->rtnl_link_ops ||
 		    dev->rtnl_link_state == RTNL_LINK_INITIALIZED)
-			skb = rtmsg_ifinfo_build_skb(RTM_DELLINK, dev, ~0U,
+			skb = rtmsg_ifinfo_build_skb(RTM_DELLINK, dev, ~0U, 0,
 						     GFP_KERNEL);
 
 		/*
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index dab2834..96eb392 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -941,6 +941,7 @@ static noinline size_t if_nlmsg_size(const struct net_device *dev,
 	       + nla_total_size(MAX_PHYS_ITEM_ID_LEN) /* IFLA_PHYS_SWITCH_ID */
 	       + nla_total_size(IFNAMSIZ) /* IFLA_PHYS_PORT_NAME */
 	       + rtnl_xdp_size() /* IFLA_XDP */
+	       + nla_total_size(4)  /* IFLA_EVENT */
 	       + nla_total_size(1); /* IFLA_PROTO_DOWN */
 
 }
@@ -1282,9 +1283,40 @@ static int rtnl_xdp_fill(struct sk_buff *skb, struct net_device *dev)
 	return err;
 }
 
+static int rtnl_fill_link_event(struct sk_buff *skb, unsigned long event)
+{
+	u32 rtnl_event;
+
+	switch (event) {
+	case NETDEV_REBOOT:
+		rtnl_event = IFLA_EVENT_REBOOT;
+		break;
+	case NETDEV_FEAT_CHANGE:
+		rtnl_event = IFLA_EVENT_FEAT_CHANGE;
+		break;
+	case NETDEV_BONDING_FAILOVER:
+		rtnl_event = IFLA_EVENT_BONDING_FAILOVER;
+		break;
+	case NETDEV_NOTIFY_PEERS:
+		rtnl_event = IFLA_EVENT_NOTIFY_PEERS;
+		break;
+	case NETDEV_RESEND_IGMP:
+		rtnl_event = IFLA_EVENT_RESEND_IGMP;
+		break;
+	case NETDEV_CHANGEINFODATA:
+		rtnl_event = IFLA_EVENT_CHANGE_INFO_DATA;
+		break;
+	default:
+		return 0;
+	}
+
+	return nla_put_u32(skb, IFLA_EVENT, rtnl_event);
+}
+
 static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
 			    int type, u32 pid, u32 seq, u32 change,
-			    unsigned int flags, u32 ext_filter_mask)
+			    unsigned int flags, u32 ext_filter_mask,
+			    unsigned long event)
 {
 	struct ifinfomsg *ifm;
 	struct nlmsghdr *nlh;
@@ -1333,6 +1365,9 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
 	    nla_put_u8(skb, IFLA_PROTO_DOWN, dev->proto_down))
 		goto nla_put_failure;
 
+	if (rtnl_fill_link_event(skb, event))
+		goto nla_put_failure;
+
 	if (rtnl_fill_link_ifmap(skb, dev))
 		goto nla_put_failure;
 
@@ -1467,6 +1502,7 @@ static const struct nla_policy ifla_policy[IFLA_MAX+1] = {
 	[IFLA_LINK_NETNSID]	= { .type = NLA_S32 },
 	[IFLA_PROTO_DOWN]	= { .type = NLA_U8 },
 	[IFLA_XDP]		= { .type = NLA_NESTED },
+	[IFLA_EVENT]		= { .type = NLA_U32 },
 };
 
 static const struct nla_policy ifla_info_policy[IFLA_INFO_MAX+1] = {
@@ -1626,7 +1662,7 @@ static int rtnl_dump_ifinfo(struct sk_buff *skb, struct netlink_callback *cb)
 					       NETLINK_CB(cb->skb).portid,
 					       cb->nlh->nlmsg_seq, 0,
 					       flags,
-					       ext_filter_mask);
+					       ext_filter_mask, 0);
 
 			if (err < 0) {
 				if (likely(skb->len))
@@ -2736,7 +2772,7 @@ static int rtnl_getlink(struct sk_buff *skb, struct nlmsghdr *nlh,
 		return -ENOBUFS;
 
 	err = rtnl_fill_ifinfo(nskb, dev, RTM_NEWLINK, NETLINK_CB(skb).portid,
-			       nlh->nlmsg_seq, 0, 0, ext_filter_mask);
+			       nlh->nlmsg_seq, 0, 0, ext_filter_mask, 0);
 	if (err < 0) {
 		/* -EMSGSIZE implies BUG in if_nlmsg_size */
 		WARN_ON(err == -EMSGSIZE);
@@ -2808,7 +2844,8 @@ static int rtnl_dump_all(struct sk_buff *skb, struct netlink_callback *cb)
 }
 
 struct sk_buff *rtmsg_ifinfo_build_skb(int type, struct net_device *dev,
-				       unsigned int change, gfp_t flags)
+				       unsigned int change,
+				       unsigned long event, gfp_t flags)
 {
 	struct net *net = dev_net(dev);
 	struct sk_buff *skb;
@@ -2819,7 +2856,7 @@ struct sk_buff *rtmsg_ifinfo_build_skb(int type, struct net_device *dev,
 	if (skb == NULL)
 		goto errout;
 
-	err = rtnl_fill_ifinfo(skb, dev, type, 0, 0, change, 0, 0);
+	err = rtnl_fill_ifinfo(skb, dev, type, 0, 0, change, 0, 0, event);
 	if (err < 0) {
 		/* -EMSGSIZE implies BUG in if_nlmsg_size() */
 		WARN_ON(err == -EMSGSIZE);
@@ -2840,18 +2877,25 @@ void rtmsg_ifinfo_send(struct sk_buff *skb, struct net_device *dev, gfp_t flags)
 	rtnl_notify(skb, net, 0, RTNLGRP_LINK, NULL, flags);
 }
 
-void rtmsg_ifinfo(int type, struct net_device *dev, unsigned int change,
-		  gfp_t flags)
+static void rtmsg_ifinfo_event(int type, struct net_device *dev,
+			       unsigned int change, unsigned long event,
+			       gfp_t flags)
 {
 	struct sk_buff *skb;
 
 	if (dev->reg_state != NETREG_REGISTERED)
 		return;
 
-	skb = rtmsg_ifinfo_build_skb(type, dev, change, flags);
+	skb = rtmsg_ifinfo_build_skb(type, dev, change, event, flags);
 	if (skb)
 		rtmsg_ifinfo_send(skb, dev, flags);
 }
+
+void rtmsg_ifinfo(int type, struct net_device *dev, unsigned int change,
+		  gfp_t flags)
+{
+	rtmsg_ifinfo_event(type, dev, change, 0, flags);
+}
 EXPORT_SYMBOL(rtmsg_ifinfo);
 
 static int nlmsg_populate_fdb_fill(struct sk_buff *skb,
@@ -4165,7 +4209,7 @@ static int rtnetlink_event(struct notifier_block *this, unsigned long event, voi
 	case NETDEV_NOTIFY_PEERS:
 	case NETDEV_RESEND_IGMP:
 	case NETDEV_CHANGEINFODATA:
-		rtmsg_ifinfo(RTM_NEWLINK, dev, 0, GFP_KERNEL);
+		rtmsg_ifinfo_event(RTM_NEWLINK, dev, 0, event, GFP_KERNEL);
 		break;
 	default:
 		break;
-- 
2.7.4

^ permalink raw reply related

* [PATCH v5 net-next 0/2] rtnetlink: Updates to rtnetlink_event()
From: Vladislav Yasevich @ 2017-05-25 15:31 UTC (permalink / raw)
  To: netdev; +Cc: dsahern, roopa, jiri, vfalico, andy, Vladislav Yasevich

This is a version 5 series came out of the conversation that started
as a result my first attempt to add netdevice event info to netlink messages.

First is the patch to add IFLA_EVENT attribute to the netlink message.  It
supports only currently white-listed events.
Like before, this is just an attribute that gets added to the rtnetlink
message only when the messaged was generated as a result of a netdev event.
In my case, this is necessary since I want to trap NETDEV_NOTIFY_PEERS
event (also possibly NETDEV_RESEND_IGMP event) and perform certain actions
in user space.  This is not possible since the messages generated as
a result of netdev events do not usually contain any changed data.  They
are just notifications.  This patch exposes this notification type to
userspace.

Second, I remove duplicate messages that a result of a change to bonding
options.  If netlink is used to configure bonding options, 2 messages
are generated, one as a result NETDEV_CHANGEINFODATA event triggered by
bonding code and one a result of device state changes triggered by
netdev_state_change (called from do_setlink).


V5: Rebased.  Added iproute2 patch to the series.
V4:
  * Removed the patch the removed NETDEV_CHANGENAME from event whitelist.
    It doesn't trigger duplicate messages since name changes can only be
    done while device is down and netdev_state_change() doesn't report
    changes while device is down.
  * Added a patch to clean-up duplicate messages on bonding option changes.

V3: Rebased.  Cleaned-up duplicate event.

V2: Added missed events (from David Ahern)

Vladislav Yasevich (2):
  rtnl: Add support for netdev event to link messages
  bonding: Prevent duplicate userspace notification

 drivers/net/bonding/bond_main.c    |  3 +-
 drivers/net/bonding/bond_options.c | 27 +++++++++++++++--
 include/linux/rtnetlink.h          |  3 +-
 include/net/bond_options.h         |  2 ++
 include/uapi/linux/if_link.h       | 11 +++++++
 net/core/dev.c                     |  2 +-
 net/core/rtnetlink.c               | 62 ++++++++++++++++++++++++++++++++------
 7 files changed, 96 insertions(+), 14 deletions(-)

-- 
2.7.4

^ permalink raw reply

* Re: [PATCH v2 net-next 1/2] Documentation: sysfs-class-net-statistics: Clarify rx_bytes and tx_bytes
From: Stephen Hemminger @ 2017-05-25 15:10 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: David Miller, Florian Fainelli, hayeswang, netdev
In-Reply-To: <1495573858-32346-2-git-send-email-andrew@lunn.ch>

On Tue, 23 May 2017 23:10:57 +0200
Andrew Lunn <andrew@lunn.ch> wrote:

> Document what is expected for the rx_bytes and tx_bytes statistics in
> /sys/class/net/<device>/statistics. The FCS should be included in the
> statistics. However, since this has been unclear until now, it is
> expected a number of drivers don't. But maybe with time they will.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  Documentation/ABI/testing/sysfs-class-net-statistics | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-net-statistics b/Documentation/ABI/testing/sysfs-class-net-statistics
> index 397118de7b5e..a487cbb79458 100644
> --- a/Documentation/ABI/testing/sysfs-class-net-statistics
> +++ b/Documentation/ABI/testing/sysfs-class-net-statistics
> @@ -21,7 +21,8 @@ Contact:	netdev@vger.kernel.org
>  Description:
>  		Indicates the number of bytes received by this network device.
>  		See the network driver for the exact meaning of when this
> -		value is incremented.
> +		value is incremented. However, for an Ethernet frame, it should
> +		include the Ethernet header, data, and frame check sum.
>  
>  What:		/sys/class/<iface>/statistics/rx_compressed
>  Date:		April 2005
> @@ -125,7 +126,8 @@ Description:
>  		device. See the network driver for the exact meaning of this
>  		value, in particular whether this accounts for all successfully
>  		transmitted packets or all packets that have been queued for
> -		transmission.
> +		transmission. For an Ethernet frame, it should include the
> +		Ethernet header, data, and frame check sum.
>  
>  What:		/sys/class/<iface>/statistics/tx_carrier_errors
>  Date:		April 2005

Sysfs statistics must match the netlink statistics. What matters is the
values reported in netlink, proc, and sysfs are the same.
Documenting the sysfs value here is nice but less important.
I worry that some naive implementer or tester might expect sysfs values
to follow different standard than other places.

For devices the important thing is that rx and tx values match. I.e if
FCS is counted on RX then it needs to get counted on Tx. I don't think
FCS should be part of data  statistics since many devices are virtual
and have no visible FCS.  For example if a packet is sent of virtio (no FCS)
to host vhost (no FCS) and then through bridge to physical hardware
which with your proposal does have FCS; all the data counts all matched.

With the advent of overlay networks this gets even more confusing.
Does overlay header count?

Linux in general has followed the BSD model of NOT including FCS
in device statistics. Some hardware vendors do include the FCS in
their stats but that is really a porting bug.

Unfortunately, even the router vendors can't agree on this.
RFC 2665 says that FCS should be included, but BSD derived systems
like Juniper do not. This is a mess.

^ permalink raw reply

* Re: [PATCH net-next] net: rps: Add the rfs_needed check when record flow hash
From: Eric Dumazet @ 2017-05-25 14:55 UTC (permalink / raw)
  To: gfree.wind; +Cc: davem, netdev
In-Reply-To: <1495611359-118199-1-git-send-email-gfree.wind@vip.163.com>

On Wed, 2017-05-24 at 15:35 +0800, gfree.wind@vip.163.com wrote:
> From: Gao Feng <gfree.wind@vip.163.com>
> 
> There are two spots which invoke the sock_rps_record_flow_hash, one is
> sock_rps_record_flow which has already checked rfs_needed. But the other
> is tun_flow_update which doesn't check if the rfs is enabled.
> 
> Now rename the original function sock_rps_record_flow_hash to
> _sock_rps_record_flow_hash, and add one helper func which checks the
> rfs_needed.
> 
> The perf result of two functions is following.
> When rfs is disabled, it could enhance 58% performance with checking
> rfs_needed.
> When rfs is enabled, the performanc is lower than current about 29%.
> 
> Because the RFS is disabled by default. I think it is useful to tun
> driver.
> 
> The follow is test statistics.

> Signed-off-by: Gao Feng <gfree.wind@vip.163.com>
> ---
>  include/net/sock.h | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/include/net/sock.h b/include/net/sock.h
> index 3467d9e..584bb9a 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -910,7 +910,7 @@ static inline void sk_incoming_cpu_update(struct sock *sk)
>  	sk->sk_incoming_cpu = raw_smp_processor_id();
>  }
>  
> -static inline void sock_rps_record_flow_hash(__u32 hash)
> +static inline void _sock_rps_record_flow_hash(__u32 hash)
>  {
>  #ifdef CONFIG_RPS
>  	struct rps_sock_flow_table *sock_flow_table;
> @@ -922,6 +922,14 @@ static inline void sock_rps_record_flow_hash(__u32 hash)
>  #endif
>  }
>  
> +static inline void sock_rps_record_flow_hash(__u32 hash)
> +{
> +#ifdef CONFIG_RPS
> +	if (static_key_false(&rfs_needed))
> +		_sock_rps_record_flow_hash(hash);
> +#endif

Seems that after your patch, sock_rps_record_flow_hash() is no longer
needed/used.

^ permalink raw reply

* Re: [PATCH net] net/mlx4: Fix the check in attaching steering rules
From: Tariq Toukan @ 2017-05-25 14:26 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <e6fc81fe-5eb6-0a6b-18df-90a018b687da@mellanox.com>



On 25/05/2017 4:07 PM, Tariq Toukan wrote:
>
> On 24/05/2017 10:36 PM, David Miller wrote:
>> From: Tariq Toukan <tariqt@mellanox.com>
>> Date: Tue, 23 May 2017 15:50:07 +0300
>>
>>> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c 
>>> b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
>>> index ae5fdc2df654..00a7cd3dcc2e 100644
>>> --- a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
>>> +++ b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
>>> @@ -1562,8 +1562,7 @@ static int mlx4_en_flow_replace(struct 
>>> net_device *dev,
>>>           qpn = priv->drop_qp.qpn;
>>>       else if (cmd->fs.ring_cookie & EN_ETHTOOL_QP_ATTACH) {
>>>           qpn = cmd->fs.ring_cookie & (EN_ETHTOOL_QP_ATTACH - 1);
>>> -        if (qpn < priv->rss_map.base_qpn ||
>>> -            qpn >= priv->rss_map.base_qpn + priv->rx_ring_num) {
>>> +        if (!mlx4_qp_lookup(priv->mdev->dev, qpn)) {
>>>               en_warn(priv, "rxnfc: QP (0x%x) doesn't exist\n", qpn);
>>>               return -EINVAL;
>>>           }
>>> diff --git a/drivers/net/ethernet/mellanox/mlx4/qp.c 
>>> b/drivers/net/ethernet/mellanox/mlx4/qp.c
>>> index 2d6abd4662b1..1eff2fe32a8b 100644
>>> --- a/drivers/net/ethernet/mellanox/mlx4/qp.c
>>> +++ b/drivers/net/ethernet/mellanox/mlx4/qp.c
>>> @@ -384,6 +384,20 @@ static void mlx4_qp_free_icm(struct mlx4_dev 
>>> *dev, int qpn)
>>>           __mlx4_qp_free_icm(dev, qpn);
>>>   }
>>>   +struct mlx4_qp *mlx4_qp_lookup(struct mlx4_dev *dev, u32 qpn)
>>   ...
>>> +EXPORT_SYMBOL_GPL(mlx4_qp_lookup);
>>> +
>>
>> This phony separation between MLX4_CORE and MLX4_EN is the only reason
>> you need this unreasonable symbol export.
>>
>> I doubt you'll ever use this function anywhere outside of en_ethtool.c
>> so this export is wasted space in the kernel image. Probably compiler
>> could inline it decently as well.
>>
>> So find another way to do this without the symbol export.  I don't
>> really want to hear any stories about "clean separation" or whatever.
>> What's happening here is exactly why this separate modules scheme
>> results in ugly unreasonable code, and unnecessary gymnastics and
>> wasted object space just to make routines available in one place from
>> another.
>
> I see. I'll remove this EXPORT_SYMBOL_GPL and send a re-spin shortly.
> Function will still be accessible within EN driver, as mlx4_en.h 
> includes qp.h.
>
Hmm, my bad, this doesn't work.
Please ignore V2.

> Regards,
> Tariq
>

^ permalink raw reply

* Re: [PATCH net V2] net/mlx4: Fix the check in attaching steering rules
From: Tariq Toukan @ 2017-05-25 14:23 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev
In-Reply-To: <1495718470-10215-1-git-send-email-tariqt@mellanox.com>


On 25/05/2017 4:21 PM, Tariq Toukan wrote:
> From: Talat Batheesh <talatb@mellanox.com>
>
> Our previous patch (cited below) introduced a regression
> for RAW Eth QPs.
>
> Fix it by checking if the QP number provided by user-space
> exists, hence allowing steering rules to be added for valid
> QPs only.
>
> Fixes: 89c557687a32 ("net/mlx4_en: Avoid adding steering rules with ...")
> Reported-by: Or Gerlitz <gerlitz.or@gmail.com>
> Signed-off-by: Talat Batheesh <talatb@mellanox.com>
> Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
> Acked-by: Or Gerlitz <ogerlitz@mellanox.com>
> Reviewed-by: Leon Romanovsky <leonro@mellanox.com>
> ---
> v2:
> * Removed unnecessary EXPORT_SYMBOL_GPL.

My bad, this doesn't work.
Please ignore patch.

>
>   drivers/net/ethernet/mellanox/mlx4/en_ethtool.c |  3 +--
>   drivers/net/ethernet/mellanox/mlx4/qp.c         | 13 +++++++++++++
>   include/linux/mlx4/qp.h                         |  1 +
>   3 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
> index ae5fdc2df654..00a7cd3dcc2e 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
> @@ -1562,8 +1562,7 @@ static int mlx4_en_flow_replace(struct net_device *dev,
>   		qpn = priv->drop_qp.qpn;
>   	else if (cmd->fs.ring_cookie & EN_ETHTOOL_QP_ATTACH) {
>   		qpn = cmd->fs.ring_cookie & (EN_ETHTOOL_QP_ATTACH - 1);
> -		if (qpn < priv->rss_map.base_qpn ||
> -		    qpn >= priv->rss_map.base_qpn + priv->rx_ring_num) {
> +		if (!mlx4_qp_lookup(priv->mdev->dev, qpn)) {
>   			en_warn(priv, "rxnfc: QP (0x%x) doesn't exist\n", qpn);
>   			return -EINVAL;
>   		}
> diff --git a/drivers/net/ethernet/mellanox/mlx4/qp.c b/drivers/net/ethernet/mellanox/mlx4/qp.c
> index 2d6abd4662b1..ad92d2311478 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/qp.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/qp.c
> @@ -384,6 +384,19 @@ static void mlx4_qp_free_icm(struct mlx4_dev *dev, int qpn)
>   		__mlx4_qp_free_icm(dev, qpn);
>   }
>   
> +struct mlx4_qp *mlx4_qp_lookup(struct mlx4_dev *dev, u32 qpn)
> +{
> +	struct mlx4_qp_table *qp_table = &mlx4_priv(dev)->qp_table;
> +	struct mlx4_qp *qp;
> +
> +	spin_lock(&qp_table->lock);
> +
> +	qp = __mlx4_qp_lookup(dev, qpn);
> +
> +	spin_unlock(&qp_table->lock);
> +	return qp;
> +}
> +
>   int mlx4_qp_alloc(struct mlx4_dev *dev, int qpn, struct mlx4_qp *qp, gfp_t gfp)
>   {
>   	struct mlx4_priv *priv = mlx4_priv(dev);
> diff --git a/include/linux/mlx4/qp.h b/include/linux/mlx4/qp.h
> index b4ee8f62ce8d..8e2828d48d7f 100644
> --- a/include/linux/mlx4/qp.h
> +++ b/include/linux/mlx4/qp.h
> @@ -470,6 +470,7 @@ struct mlx4_update_qp_params {
>   	u16	rate_val;
>   };
>   
> +struct mlx4_qp *mlx4_qp_lookup(struct mlx4_dev *dev, u32 qpn);
>   int mlx4_update_qp(struct mlx4_dev *dev, u32 qpn,
>   		   enum mlx4_update_qp_attr attr,
>   		   struct mlx4_update_qp_params *params);

^ 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