Netdev List
 help / color / mirror / Atom feed
* [GIT] Networking
From: David Miller @ 2015-02-17 18:48 UTC (permalink / raw)
  To: torvalds; +Cc: akpm, netdev, linux-kernel


1) Missing netlink attribute validation in nft_lookup, from Patrick
   McHardy.

2) Restrict ipv6 partial checksum handling to UDP, since that's the
   only case it works for.  From Vlad Yasevich.

3) Clear out silly device table sentinal macros used by SSB and BCMA
   drivers.  From Joe Perches.

4) Make sure the remote checksum code never creates a situation where
   the remote checksum is applied yet the tunneling metadata describing
   the remote checksum transformation is still present.  Otherwise
   an external entity might see this and apply the checksum again.
   From Tom Herbert.

5) Use msecs_to_jiffies() where applicable, from Nicholas Mc Guire.

6) Don't explicitly initialize timer struct fields, use setup_timer()
   and mod_timer() instead.  From Vaishali Thakkar.

7) Don't invoke tg3_halt() without the tp->lock held, from Jun'ichi
   Nomura.

8) Missing __percpu annotation in ipvlan driver, from Eric Dumazet.

9) Don't potentially perform skb_get() on shared skbs, also from
   Eric Dumazet.

10) Fix COW'ing of metrics for non-DST_HOST routes in ipv6, from
    Martin KaFai Lau.

11) Fix merge resolution error between the iov_iter changes in
    vhost and some bug fixes that occurred at the same time.
    From Jason Wang.

12) If rtnl_configure_link() fails we have to perform a call to
    ->dellink() before unregistering the device.  From WANG Cong.

Please pull, thanks a lot!

The following changes since commit 73b4f63aebd6d57db4ca1d31fa6f8516651207b0:

  Merge tag 'docs-for-linus' of git://git.lwn.net/linux-2.6 (2015-02-11 13:03:11 -0800)

are available in the git repository at:


  git://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git master

for you to fetch changes up to 19334920eaf7df3f69950b040ede6c7598425a5b:

  net: dsa: Set valid phy interface type (2015-02-17 10:37:39 -0800)

----------------------------------------------------------------
Aleksander Morgado (1):
      hso: fix rx parsing logic when skb allocation fails

Arturo Borrero (1):
      netfilter: nft_compat: add ebtables support

David L Stevens (1):
      sunvnet: don't change gso data on clones

David S. Miller (3):
      Merge git://git.kernel.org/.../pablo/nf-next
      Merge branch 'rco_correctness'
      Merge tag 'wireless-drivers-for-davem-2015-02-11' of git://git.kernel.org/.../kvalo/wireless-drivers

Eli Cohen (1):
      net/mlx5_core: Fix configuration of log_uar_page_sz

Eric Dumazet (2):
      ipvlan: add a missing __percpu pcpu_stats
      tcp: make sure skb is not shared before using skb_get()

Geert Uytterhoeven (3):
      openvswitch: Add missing initialization in validate_and_copy_set_tun()
      drivers: net: xgene: Make xgene_enet_of_match depend on CONFIG_OF
      bridge: netfilter: Move sysctl-specific error code inside #ifdef

Guenter Roeck (1):
      net: dsa: Set valid phy interface type

Jan Stancek (1):
      ipv6: fix possible deadlock in ip6_fl_purge / ip6_fl_gc

Jason Wang (1):
      vhost_net: fix wrong iter offset when setting number of buffers

Joe Perches (2):
      treewide: Remove unnecessary BCMA_CORETABLE_END macro
      treewide: Remove unnecessary SSB_DEVTABLE_END macro

Jun'ichi Nomura \(NEC\) (1):
      tg3: Hold tp->lock before calling tg3_halt() from tg3_init_one()

Larry Finger (1):
      rtlwifi: Remove logging statement that is no longer needed

Martin KaFai Lau (1):
      ipv6: fix ipv6_cow_metrics for non DST_HOST case

Masanari Iida (1):
      net/core: Fix warning while make xmldocs caused by dev.c

Michael Grzeschik (1):
      com20020-pci: add support for eae single card

Nicholas Mc Guire (1):
      et131x: use msecs_to_jiffies for conversions

Patrick McHardy (1):
      netfilter: nft_lookup: add missing attribute validation for NFTA_LOOKUP_SET_ID

Pravin B Shelar (2):
      openvswitch: Reset key metadata for packet execution.
      openvswitch: Fix key serialization.

Rafał Miłecki (1):
      bgmac: fix device initialization on Northstar SoCs (condition typo)

Shahed Shaikh (1):
      qlcnic: Delete existing multicast MAC list before adding new

Sowmini Varadhan (1):
      rds: rds_cong_queue_updates needs to defer the congestion update transmission

Stephen Hemminger (1):
      net: spelling fixes

Sylvain Rochet (1):
      net: phy: micrel: disable NAND-tree for KSZ8021, KSZ8031, KSZ8051, KSZ8081

Tom Herbert (7):
      net: Fix remcsum in GRO path to not change packet
      net: Clarify meaning of CHECKSUM_PARTIAL for receive path
      udp: Set SKB_GSO_UDP_TUNNEL* in UDP GRO path
      net: Use more bit fields in napi_gro_cb
      net: Infrastructure for CHECKSUM_PARTIAL with remote checsum offload
      vxlan: Use checksum partial with remote checksum offload
      gue: Use checksum partial with remote checksum offload

Vaishali Thakkar (1):
      drivers/net: Use setup_timer and mod_timer

Vlad Yasevich (1):
      ipv6: Partial checksum only UDP packets

WANG Cong (1):
      rtnetlink: call ->dellink on failure when ->newlink exists

hayeswang (1):
      r8152: restore hw settings

 drivers/net/arcnet/com20020-pci.c                        |   21 ++++++++++++++++++---
 drivers/net/ethernet/3com/3c589_cs.c                     |    7 ++-----
 drivers/net/ethernet/agere/et131x.c                      |    6 ++++--
 drivers/net/ethernet/apm/xgene/xgene_enet_main.c         |    2 ++
 drivers/net/ethernet/broadcom/b44.c                      |    2 +-
 drivers/net/ethernet/broadcom/bgmac.c                    |    7 ++++---
 drivers/net/ethernet/broadcom/tg3.c                      |    2 ++
 drivers/net/ethernet/mellanox/mlx5/core/main.c           |    1 +
 drivers/net/ethernet/qlogic/qlcnic/qlcnic.h              |   11 ++++++++++-
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_hw.c           |   34 +++++++++++++++++++++++++++++-----
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_common.c |   19 ++++++++++++-------
 drivers/net/ethernet/sun/sunvnet.c                       |   23 ++++++++++-------------
 drivers/net/ipvlan/ipvlan.h                              |    2 +-
 drivers/net/phy/micrel.c                                 |   28 ++++++++++++++++++++++++++++
 drivers/net/usb/hso.c                                    |    2 +-
 drivers/net/usb/r8152.c                                  |   59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 drivers/net/vxlan.c                                      |   38 +++++++++++++++++++++++++-------------
 drivers/net/wireless/b43/main.c                          |    4 ++--
 drivers/net/wireless/b43legacy/main.c                    |    2 +-
 drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c    |    2 +-
 drivers/net/wireless/rtlwifi/pci.c                       |    5 +----
 drivers/spi/spi-bcm53xx.c                                |    2 +-
 drivers/ssb/driver_gige.c                                |    2 +-
 drivers/usb/host/bcma-hcd.c                              |    2 +-
 drivers/usb/host/ssb-hcd.c                               |    2 +-
 drivers/vhost/net.c                                      |   11 ++++++-----
 include/linux/mod_devicetable.h                          |    4 ----
 include/linux/netdevice.h                                |   62 ++++++++++++++++++++++++++++++++++++++++++++++++++++----------
 include/linux/skbuff.h                                   |   32 ++++++++++++++++++++++++++------
 include/net/checksum.h                                   |    5 +++++
 include/net/vxlan.h                                      |    4 +++-
 include/uapi/linux/fou.h                                 |    1 +
 include/uapi/linux/if_link.h                             |    1 +
 net/bridge/br_netfilter.c                                |    7 ++-----
 net/core/dev.c                                           |    3 ++-
 net/core/filter.c                                        |    2 +-
 net/core/pktgen.c                                        |    2 +-
 net/core/rtnetlink.c                                     |    9 ++++++++-
 net/dsa/slave.c                                          |    9 +++++++--
 net/ipv4/devinet.c                                       |    2 +-
 net/ipv4/fou.c                                           |   42 +++++++++++++++++++++++++++++-------------
 net/ipv4/tcp_fastopen.c                                  |   32 ++++++++++++++++++++++++--------
 net/ipv4/udp_offload.c                                   |   13 ++++++++++++-
 net/ipv6/ip6_flowlabel.c                                 |    4 ++--
 net/ipv6/ip6_output.c                                    |    2 +-
 net/ipv6/route.c                                         |    2 +-
 net/ipv6/udp_offload.c                                   |    6 +++++-
 net/netfilter/nft_compat.c                               |   63 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++------
 net/netfilter/nft_lookup.c                               |    1 +
 net/openvswitch/flow.c                                   |    2 ++
 net/openvswitch/flow_netlink.c                           |    4 ++--
 net/rds/cong.c                                           |   16 +++++++++++++++-
 52 files changed, 483 insertions(+), 143 deletions(-)

^ permalink raw reply

* Re: [PATCH] net: dsa: Set valid phy interface type
From: David Miller @ 2015-02-17 18:37 UTC (permalink / raw)
  To: linux; +Cc: netdev, f.fainelli, andrew
In-Reply-To: <1424150631-22396-1-git-send-email-linux@roeck-us.net>

From: Guenter Roeck <linux@roeck-us.net>
Date: Mon, 16 Feb 2015 21:23:51 -0800

> If the phy interface mode is not found in devicetree, or if devicetree
> is not configured, of_get_phy_mode returns -ENODEV. The current code
> sets the phy interface mode to the return value from of_get_phy_mode
> without checking if it is valid.
> 
> This invalid phy interface mode is passed as parameter to of_phy_connect
> or to phy_connect_direct. This sets the phy interface mode to the invalid
> value, which in turn causes problems for any code using phydev->interface.
> 
> Fixes: b31f65fb4383 ("net: dsa: slave: Fix autoneg for phys on switch MDIO bus")
> Fixes: 0d8bcdd383b8 ("net: dsa: allow for more complex PHY setups")
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH net] ematch: Fix auto-loading of ematch modules.
From: Cong Wang @ 2015-02-17 18:00 UTC (permalink / raw)
  To: Ignacy Gawędzki, Cong Wang, netdev
In-Reply-To: <20150216185011.GA29935@zenon.in.qult.net>

On Mon, Feb 16, 2015 at 10:50 AM, Ignacy Gawędzki
<ignacy.gawedzki@green-communications.fr> wrote:
> On Mon, Feb 16, 2015 at 10:36:06AM -0800, thus spake Cong Wang:
>> On Mon, Feb 16, 2015 at 10:13 AM, Ignacy Gawędzki
>> <ignacy.gawedzki@green-communications.fr> wrote:
>> > In tcf_em_validate(), when calling tcf_em_lookup(), don't put the resulting
>> > pointer directly into em->ops, if the function is to possibly return -EAGAIN
>> > after module auto-loading.  Otherwise, module_put() will be called by
>> > tcf_em_tree_destroy() on em->ops->owner, while it has already been called by
>> > tcf_em_validate() before return.
>> >
>>
>> Or simply reset em->ops to NULL after module_put()?
>
> Yeah, why not.  I just got my inspiration from what's done in cls_api.c and
> act_api.c.  What do you think, should the same simplification be applied to
> cls_api.c?
>

I prefer the smaller change, we don't have to align with cls_api.c.
Yeah, you can always clean it up for net-next.

Thanks.

^ permalink raw reply

* [PATCH] net: phy: Fix verification of EEE support in phy_init_eee
From: Guenter Roeck @ 2015-02-17 17:36 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: netdev, Guenter Roeck, Giuseppe Cavallaro

phy_init_eee uses phy_find_setting(phydev->speed, phydev->duplex)
to find a valid entry in the settings array for the given speed
and duplex value. For full duplex 1000baseT, this will return
the first matching entry, which is the entry for 1000baseKX_Full.

If the phy eee does not support 1000baseKX_Full, this entry will not
match, causing phy_init_eee to fail for no good reason.

Fixes: 9a9c56cb34e6 ("net: phy: fix a bug when verify the EEE support")
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
I have another version where phy_check_valid() is open coded. It looks a bit
messy to me, but I don't really mind either way if that is preferred.

 drivers/net/phy/phy.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index cdcac6a..52cd8db 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -236,6 +236,25 @@ static inline unsigned int phy_find_valid(unsigned int idx, u32 features)
 }
 
 /**
+ * phy_check_valid - check if there is a valid PHY setting which matches
+ *		     speed, duplex, and feature mask
+ * @speed: speed to match
+ * @duplex: duplex to match
+ * @features: A mask of the valid settings
+ *
+ * Description: Returns true if there is a valid setting, false otherwise.
+ */
+static inline bool phy_check_valid(int speed, int duplex, u32 features)
+{
+	unsigned int idx;
+
+	idx = phy_find_valid(phy_find_setting(speed, duplex), features);
+
+	return settings[idx].speed == speed && settings[idx].duplex == duplex &&
+		(settings[idx].setting & features);
+}
+
+/**
  * phy_sanitize_settings - make sure the PHY is set to supported speed and duplex
  * @phydev: the target phy_device struct
  *
@@ -1045,7 +1064,6 @@ int phy_init_eee(struct phy_device *phydev, bool clk_stop_enable)
 		int eee_lp, eee_cap, eee_adv;
 		u32 lp, cap, adv;
 		int status;
-		unsigned int idx;
 
 		/* Read phy status to properly get the right settings */
 		status = phy_read_status(phydev);
@@ -1077,8 +1095,7 @@ int phy_init_eee(struct phy_device *phydev, bool clk_stop_enable)
 
 		adv = mmd_eee_adv_to_ethtool_adv_t(eee_adv);
 		lp = mmd_eee_adv_to_ethtool_adv_t(eee_lp);
-		idx = phy_find_setting(phydev->speed, phydev->duplex);
-		if (!(lp & adv & settings[idx].setting))
+		if (!phy_check_valid(phydev->speed, phydev->duplex, lp & adv))
 			goto eee_exit_err;
 
 		if (clk_stop_enable) {
-- 
2.1.0

^ permalink raw reply related

* Re: [PATCH v2 1/3] Ethernet packet sniffer: Device tree binding and vendor prefix
From: Mark Rutland @ 2015-02-17 17:30 UTC (permalink / raw)
  To: Stathis Voukelatos
  Cc: linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	netdev@vger.kernel.org, abrestic@chromium.org
In-Reply-To: <54E376AF.80602@linn.co.uk>

On Tue, Feb 17, 2015 at 05:13:19PM +0000, Stathis Voukelatos wrote:
> Hi Mark,
> 
> On 17/02/15 16:35, Mark Rutland wrote:
> >
> >>>> +- tstamp-hz : frequency of the timestamp counter
> >
> > Is this the frequency the clock is running at, or a frequency that it
> > should be programmed to in order to be used?
> >
> > The former can be queried from the common clock framework, and if you
> > intended the latter the wording shuold be a little more explicit about
> > that being the case.
> >
> 
> It is the frequency of the timestamp values supplied to the sniffer
> module. It is used by the driver to convert to nanoseconds.
> I was trying to be somewhat generic here and not assume that it
> is necessarily the same as the 'tstamp' clock below, in which case we
> could indeed obtain it using the common clock framework.

In what cases would it _not_ be the same? From your description this is
that clock, no?

> >> See: include/linux/clocksource.h
> >> The driver uses a cyclecounter and timecounter to convert raw timestamps
> >> to nanoseconds. 'tstamp-shift' refers to the 'shift' field of the
> >> cyclecounter structure, that can be used to improve the precision of
> >> the conversion
> >
> > Sure, but the very concept of a cyclecounter is a Linux implementation
> > detail. If we have the frequency of the timer we should be able to
> > dynamically generate this, so there's no need for this to be in the DT.
> >
> 
> Most networking driver use hard-coded values for that, but in my case
> I did not want to assume a certain fixed clock frequency. I will remove
> it from the DT and generate it dynamically. There is a kernel function
> clocks_calc_mult_shift() to do it but unfortunately it is not exported,
> so I guess I will need to replicate the code.

Or submit a patch exporting it, along with the rationale for doing so?

> >>> As mentioned previously, I think the relation between this unit and the
> >>> MAC and/or PHY needs to be explicitly described in the DT.
> >>
> >> Do you suggest a field along the lines of:
> >> mac = <&eth_controller_0>;
> >> The driver could check that it exists and is valid but does
> >> not need to make use of it.
> >
> > I would expect some level of the software stack to make use of it, or
> > you have no idea which ethernet interface is related to this monitoring
> > interface. Perhaps current systems only have one interface, but that
> > shouldn't be relied upon.
> 
> Yes, but the sniffer module is hard-wired to a certain Ethernet Mii
> interface. We can add an entry to tie it to an Ethernet controller, but
> apart of a sanity check I am not sure what else the S/W can do.

Fundamentally, the use-case for this is monitoring an ethernet
interface. So regardless of which kernel framework this plumbs into,
there needs to be a way to go from ethN to whatever this is exposed as.

Exposing a completely separate interface makes no sense. Singleton stuff
like that inevitably gets broken as someone later builds a board with
multiple instances of some similar IP block.

So I would imagine that either the link between interface and monitoring
interface would be described somewhere in the filesystem, or there'd be
a syscall/ioctl/whatever to go from an interface to the appropriate
monitoring interface.

That all depends on exactly how this gets exposed in the end, however.

Thanks,
Mark.

^ permalink raw reply

* Re: [PATCH v2 1/3] Ethernet packet sniffer: Device tree binding and vendor prefix
From: Stathis Voukelatos @ 2015-02-17 17:13 UTC (permalink / raw)
  To: Mark Rutland
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	abrestic-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org
In-Reply-To: <20150217163527.GR8994@leverpostej>

Hi Mark,

On 17/02/15 16:35, Mark Rutland wrote:
>
>>>> +- tstamp-hz : frequency of the timestamp counter
>
> Is this the frequency the clock is running at, or a frequency that it
> should be programmed to in order to be used?
>
> The former can be queried from the common clock framework, and if you
> intended the latter the wording shuold be a little more explicit about
> that being the case.
>

It is the frequency of the timestamp values supplied to the sniffer
module. It is used by the driver to convert to nanoseconds.
I was trying to be somewhat generic here and not assume that it
is necessarily the same as the 'tstamp' clock below, in which case we
could indeed obtain it using the common clock framework.

>> See: include/linux/clocksource.h
>> The driver uses a cyclecounter and timecounter to convert raw timestamps
>> to nanoseconds. 'tstamp-shift' refers to the 'shift' field of the
>> cyclecounter structure, that can be used to improve the precision of
>> the conversion
>
> Sure, but the very concept of a cyclecounter is a Linux implementation
> detail. If we have the frequency of the timer we should be able to
> dynamically generate this, so there's no need for this to be in the DT.
>

Most networking driver use hard-coded values for that, but in my case
I did not want to assume a certain fixed clock frequency. I will remove
it from the DT and generate it dynamically. There is a kernel function
clocks_calc_mult_shift() to do it but unfortunately it is not exported,
so I guess I will need to replicate the code.

>>> As mentioned previously, I think the relation between this unit and the
>>> MAC and/or PHY needs to be explicitly described in the DT.
>>
>> Do you suggest a field along the lines of:
>> mac = <&eth_controller_0>;
>> The driver could check that it exists and is valid but does
>> not need to make use of it.
>
> I would expect some level of the software stack to make use of it, or
> you have no idea which ethernet interface is related to this monitoring
> interface. Perhaps current systems only have one interface, but that
> shouldn't be relied upon.

Yes, but the sniffer module is hard-wired to a certain Ethernet Mii
interface. We can add an entry to tie it to an Ethernet controller, but
apart of a sanity check I am not sure what else the S/W can do.

>
> Thanks,
> Mark.
>

Thank you,
Stathis
--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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] vhost: support upto 509 memory regions
From: Paolo Bonzini @ 2015-02-17 17:09 UTC (permalink / raw)
  To: Igor Mammedov, Michael S. Tsirkin; +Cc: linux-kernel, kvm, netdev
In-Reply-To: <20150217160240.1085e2f7@nial.brq.redhat.com>



On 17/02/2015 16:02, Igor Mammedov wrote:
>> > 
>> > Not if there are about 6 regions, I think.
> When memslots where increased to 509 and look up of them was replaced on
> binary search results were on par with linear search for a default 13 memslots VM.
> 
> Adding LRU

You mean MRU. :)

> cache helped to shave ~40% of cycles for sequential lookup workloads.

It's a bit different for vhost because you can have up to four "things"
being looked up at the same time:

- the s/g list that will end up in the skb

- the avail/used ring

- the virtio buffers

- the virtio indirect buffers

So you probably need multiple MRU caches.  But yes, MRU can help a lot.

Paolo

^ permalink raw reply

* Re: [PATCH] net: dsa: Set valid phy interface type
From: Florian Fainelli @ 2015-02-17 17:04 UTC (permalink / raw)
  To: Guenter Roeck, David S. Miller; +Cc: netdev, Andrew Lunn
In-Reply-To: <1424150631-22396-1-git-send-email-linux@roeck-us.net>

On 16/02/15 21:23, Guenter Roeck wrote:
> If the phy interface mode is not found in devicetree, or if devicetree
> is not configured, of_get_phy_mode returns -ENODEV. The current code
> sets the phy interface mode to the return value from of_get_phy_mode
> without checking if it is valid.
> 
> This invalid phy interface mode is passed as parameter to of_phy_connect
> or to phy_connect_direct. This sets the phy interface mode to the invalid
> value, which in turn causes problems for any code using phydev->interface.
> 
> Fixes: b31f65fb4383 ("net: dsa: slave: Fix autoneg for phys on switch MDIO bus")
> Fixes: 0d8bcdd383b8 ("net: dsa: allow for more complex PHY setups")
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>

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

This is 'net' material since you are fixing a bug here, thanks!

> ---
>  net/dsa/slave.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> index d104ae1..f23dead 100644
> --- a/net/dsa/slave.c
> +++ b/net/dsa/slave.c
> @@ -521,10 +521,13 @@ static int dsa_slave_phy_setup(struct dsa_slave_priv *p,
>  	struct device_node *phy_dn, *port_dn;
>  	bool phy_is_fixed = false;
>  	u32 phy_flags = 0;
> -	int ret;
> +	int mode, ret;
>  
>  	port_dn = cd->port_dn[p->port];
> -	p->phy_interface = of_get_phy_mode(port_dn);
> +	mode = of_get_phy_mode(port_dn);
> +	if (mode < 0)
> +		mode = PHY_INTERFACE_MODE_NA;
> +	p->phy_interface = mode;
>  
>  	phy_dn = of_parse_phandle(port_dn, "phy-handle", 0);
>  	if (of_phy_is_fixed_link(port_dn)) {
> @@ -559,6 +562,8 @@ static int dsa_slave_phy_setup(struct dsa_slave_priv *p,
>  		if (!p->phy)
>  			return -ENODEV;
>  
> +		/* Use already configured phy mode */
> +		p->phy_interface = p->phy->interface;
>  		phy_connect_direct(slave_dev, p->phy, dsa_slave_adjust_link,
>  				   p->phy_interface);
>  	} else {
> 


-- 
Florian

^ permalink raw reply

* Re: [PATCH v2 1/3] Ethernet packet sniffer: Device tree binding and vendor prefix
From: Mark Rutland @ 2015-02-17 16:35 UTC (permalink / raw)
  To: Stathis Voukelatos
  Cc: linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	netdev@vger.kernel.org, abrestic@chromium.org
In-Reply-To: <54E36AAC.7010308@linn.co.uk>

On Tue, Feb 17, 2015 at 04:22:04PM +0000, Stathis Voukelatos wrote:
> Hi Mark,
> 
> On 17/02/15 14:51, Mark Rutland wrote:
> 
> >> +Matched packet bytes and timestamp values are returned through a
> >> +FIFO. Timestamps are provided to the module through an externally
> >> +generated Gray-encoded counter.
> >
> > Does this counter unit need to be enabled (or have any input clocks
> > enabled)?
> >
> 
> Yes it does, that is the purpose of the 'tstamp' entry in the
> 'clock-names' property below.

Ah, I see.

> >> +
> >> +Required properties:
> >> +- compatible : must be "linn,eth-sniffer"
> >
> > Is there not a more precise name for this IP block?
> 
> It is generally called 'ethernet packet sniffer', maybe
> linn,eth-packet-sniffer would be a more descriptive name?

Either way is fine. I was expecting something more like a product code.

[...]

> >> +- fifo-block-words : number of words in one data FIFO entry
> >
> > I didn't see a data FIFO described. Is that dynamically allocated and
> > handed to the sniffer, or does that correspond to one of the memory
> > regions above?
> >
> 
> It is a H/W FIFO internal to the module and accessed through
> a register. It is divided in blocks and 'fifo-block-words'
> specify the number of words in each block. It is needed by
> the driver to make sure it reads an entire block, in order
> to clear the 'data available' interrupt.

I see. I assumed that the FIFO was memory mapped rather than exposed
through a register.

Given the above this sounds fine.

> >> +- tstamp-hz : frequency of the timestamp counter

Is this the frequency the clock is running at, or a frequency that it
should be programmed to in order to be used?

The former can be queried from the common clock framework, and if you
intended the latter the wording shuold be a little more explicit about
that being the case.

> >> +- tstamp-shift : shift value for the timestamp cyclecounter struct
> >
> > What exactly is this used for?
> >
> > Are there any docs?
> 
> See: include/linux/clocksource.h
> The driver uses a cyclecounter and timecounter to convert raw timestamps
> to nanoseconds. 'tstamp-shift' refers to the 'shift' field of the
> cyclecounter structure, that can be used to improve the precision of
> the conversion

Sure, but the very concept of a cyclecounter is a Linux implementation
detail. If we have the frequency of the timer we should be able to
dynamically generate this, so there's no need for this to be in the DT.

> >> +- tstamp-bits : width in bits of the timestamp counter
> >> +
> >> +Example:
> >> +
> >> +sniffer@1814a000 {
> >> +	compatible = "linn,eth-sniffer";
> >> +	reg = <0x1814a000 0x100>, <0x1814a400 0x400>,
> >> +	      <0x1814a800 0x400>;
> >> +	reg-names = "regs", "tx-ram", "rx-ram";
> >> +	interrupts = <GIC_SHARED 58 IRQ_TYPE_LEVEL_HIGH>;
> >> +	clocks = <&clk_core CLK_AUDIO>,
> >> +		 <&cr_periph SYS_CLK_EVENT_TIMER>;
> >> +	clock-names = "sys", "tstamp";
> >> +	fifo-block-words = <4>;
> >> +	tstamp-hz = <52000000>;
> >> +	tstamp-shift = <27>;
> >> +	tstamp-bits = <30>;
> >
> > This property wasn't documented.
> >
> > As mentioned previously, I think the relation between this unit and the
> > MAC and/or PHY needs to be explicitly described in the DT.
> 
> Do you suggest a field along the lines of:
> mac = <&eth_controller_0>;
> The driver could check that it exists and is valid but does
> not need to make use of it.

I would expect some level of the software stack to make use of it, or
you have no idea which ethernet interface is related to this monitoring
interface. Perhaps current systems only have one interface, but that
shouldn't be relied upon.

Thanks,
Mark.

^ permalink raw reply

* Re: [PATCH]     iproute2: Extend ip address command to enable multicast group join/leave on IP level.
From: Stephen Hemminger @ 2015-02-17 16:34 UTC (permalink / raw)
  To: Madhu Challa; +Cc: netdev
In-Reply-To: <1424125415-4616-1-git-send-email-challa@noironetworks.com>

On Mon, 16 Feb 2015 14:23:35 -0800
Madhu Challa <challa@noironetworks.com> wrote:

>     Joining multicast group on ethernet level via "ip maddr" command would
>     not work if we have an Ethernet switch that does igmp snooping since
>     the switch would not replicate multicast packets on ports that did not
>     have IGMP reports for the multicast addresses.
> 
>     Linux vxlan interfaces created via "ip link add vxlan" have the group option
>     that enables then to do the required join.
> 
>     By extending ip address command with option "autojoin" we can get similar
>     functionality for openvswitch vxlan interfaces as well as other tunneling
>     mechanisms that need to receive multicast traffic. The kernel code is
>     structured similar to how the vxlan driver does a group join / leave.
> 
>     example:
>     ip address add 224.1.1.10/24 dev eth5 autojoin
>     ip address del 224.1.1.10/24 dev eth5
> 
> Signed-off-by: Madhu Challa <challa@noironetworks.com>

You need to get MCAUTOJOIN into net-next first.

^ permalink raw reply

* [PATCH v3 iproute2 3/4] iplink: add support of IFLA_LINK_NETNSID attribute
From: Nicolas Dichtel @ 2015-02-17 16:30 UTC (permalink / raw)
  To: shemminger; +Cc: netdev, Nicolas Dichtel
In-Reply-To: <1424190639-5333-1-git-send-email-nicolas.dichtel@6wind.com>

This new attribute is now advertised by the kernel for x-netns interfaces.
It's also possible to set it when an interface is created (and thus creating a
x-netns interface with one single message).

Example:
 $ ip netns add foo
 $ ip netns add bar
 $ ip -n foo netns set bar 15
 $ ip -n foo link add ipip1 link-netnsid 15 type ipip remote 10.16.0.121 local 10.16.0.249
 $ ip -n foo link ls ipip1
 3: ipip1@NONE: <POINTOPOINT,NOARP> mtu 1480 qdisc noop state DOWN mode DEFAULT group default
     link/ipip 10.16.0.249 peer 10.16.0.121 link-netnsid 15

Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
v3: rebase on master branch
v2: don't try to convert IFLA_LINK to the devname when IFLA_LINK_NETNSID is set

 ip/ipaddress.c | 20 +++++++++++++++++---
 ip/iplink.c    | 10 ++++++++++
 2 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 3730424ad8cd..99a6ab5977e3 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -28,6 +28,7 @@
 #include <linux/netdevice.h>
 #include <linux/if_arp.h>
 #include <linux/sockios.h>
+#include <linux/net_namespace.h>
 
 #include "rt_names.h"
 #include "utils.h"
@@ -614,9 +615,13 @@ int print_linkinfo(const struct sockaddr_nl *who,
 		if (iflink == 0)
 			fprintf(fp, "@NONE: ");
 		else {
-			fprintf(fp, "@%s: ", ll_idx_n2a(iflink, b1));
-			m_flag = ll_index_to_flags(iflink);
-			m_flag = !(m_flag & IFF_UP);
+			if (tb[IFLA_LINK_NETNSID])
+				fprintf(fp, "@if%d: ", iflink);
+			else {
+				fprintf(fp, "@%s: ", ll_idx_n2a(iflink, b1));
+				m_flag = ll_index_to_flags(iflink);
+				m_flag = !(m_flag & IFF_UP);
+			}
 		}
 	} else {
 		fprintf(fp, ": ");
@@ -678,6 +683,15 @@ int print_linkinfo(const struct sockaddr_nl *who,
 		}
 	}
 
+	if (tb[IFLA_LINK_NETNSID]) {
+		int id = *(int*)RTA_DATA(tb[IFLA_LINK_NETNSID]);
+
+		if (id >= 0)
+			fprintf(fp, " link-netnsid %d", id);
+		else
+			fprintf(fp, " link-netnsid unknown");
+	}
+
 	if (tb[IFLA_PROMISCUITY] && show_details)
 		fprintf(fp, " promiscuity %u ",
 			*(int*)RTA_DATA(tb[IFLA_PROMISCUITY]));
diff --git a/ip/iplink.c b/ip/iplink.c
index c93d1dc3d5f6..5893ee401cf9 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -72,6 +72,7 @@ void iplink_usage(void)
 	fprintf(stderr, "	                  [ mtu MTU ]\n");
 	fprintf(stderr, "	                  [ netns PID ]\n");
 	fprintf(stderr, "	                  [ netns NAME ]\n");
+	fprintf(stderr, "                         [ link-netnsid ID ]\n");
 	fprintf(stderr, "			  [ alias NAME ]\n");
 	fprintf(stderr, "	                  [ vf NUM [ mac LLADDR ]\n");
 	fprintf(stderr, "				   [ vlan VLANID [ qos VLAN-QOS ] ]\n");
@@ -386,6 +387,7 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
 	int numtxqueues = -1;
 	int numrxqueues = -1;
 	int dev_index = 0;
+	int link_netnsid = -1;
 
 	*group = -1;
 	ret = argc;
@@ -588,6 +590,14 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
 			addattr8(&req->n, sizeof(*req), IFLA_INET6_ADDR_GEN_MODE, mode);
 			addattr_nest_end(&req->n, afs6);
 			addattr_nest_end(&req->n, afs);
+		} else if (matches(*argv, "link-netnsid") == 0) {
+			NEXT_ARG();
+			if (link_netnsid != -1)
+				duparg("link-netnsid", *argv);
+			if (get_integer(&link_netnsid, *argv, 0))
+				invarg("Invalid \"link-netnsid\" value\n", *argv);
+			addattr32(&req->n, sizeof(*req), IFLA_LINK_NETNSID,
+				  link_netnsid);
 		} else {
 			if (strcmp(*argv, "dev") == 0) {
 				NEXT_ARG();
-- 
2.2.2

^ permalink raw reply related

* [PATCH v3 iproute2 4/4] bridge/fdb: display link netns id
From: Nicolas Dichtel @ 2015-02-17 16:30 UTC (permalink / raw)
  To: shemminger; +Cc: netdev, Nicolas Dichtel
In-Reply-To: <1424190639-5333-1-git-send-email-nicolas.dichtel@6wind.com>

When this attribute is set, it means that the i/o part of the related netdevice
is in another netns.

Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
v3: rebase on master branch
v2: new in the series

 bridge/fdb.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/bridge/fdb.c b/bridge/fdb.c
index c01a5020de63..6941edd90f90 100644
--- a/bridge/fdb.c
+++ b/bridge/fdb.c
@@ -131,12 +131,16 @@ int print_fdb(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 		if (ifindex) {
 			char ifname[IF_NAMESIZE];
 
-			if (if_indextoname(ifindex, ifname))
+			if (!tb[NDA_LINK_NETNSID] &&
+			    if_indextoname(ifindex, ifname))
 				fprintf(fp, "via %s ", ifname);
 			else
 				fprintf(fp, "via ifindex %u ", ifindex);
 		}
 	}
+	if (tb[NDA_LINK_NETNSID])
+		fprintf(fp, "link-netnsid %d ",
+			rta_getattr_u32(tb[NDA_LINK_NETNSID]));
 
 	if (show_stats && tb[NDA_CACHEINFO]) {
 		struct nda_cacheinfo *ci = RTA_DATA(tb[NDA_CACHEINFO]);
-- 
2.2.2

^ permalink raw reply related

* [PATCH v3 iproute2 2/4] ipnetns: allow to get and set netns ids
From: Nicolas Dichtel @ 2015-02-17 16:30 UTC (permalink / raw)
  To: shemminger; +Cc: netdev, Nicolas Dichtel
In-Reply-To: <1424190639-5333-1-git-send-email-nicolas.dichtel@6wind.com>

The kernel now provides ids for peer netns. This patch implements a new command
'set' to assign an id.
When netns are listed, if an id is assigned, it is now displayed.

Example:
 $ ip netns add foo
 $ ip netns set foo 1
 $ ip netns
 foo (id: 1)
 init_net

Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
v3: rebase on master branch
v2: no change

 include/libnetlink.h |   8 ++++
 ip/ipnetns.c         | 113 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 man/man8/ip-netns.8  |  14 +++++++
 3 files changed, 134 insertions(+), 1 deletion(-)

diff --git a/include/libnetlink.h b/include/libnetlink.h
index d081e54210ea..898275b824d4 100644
--- a/include/libnetlink.h
+++ b/include/libnetlink.h
@@ -158,6 +158,14 @@ extern int rtnl_from_file(FILE *, rtnl_filter_t handler,
 #define NDTA_PAYLOAD(n) NLMSG_PAYLOAD(n,sizeof(struct ndtmsg))
 #endif
 
+#ifndef NETNS_RTA
+#define NETNS_RTA(r) \
+	((struct rtattr*)(((char*)(r)) + NLMSG_ALIGN(sizeof(struct rtgenmsg))))
+#endif
+#ifndef NETNS_PAYLOAD
+#define NETNS_PAYLOAD(n)	NLMSG_PAYLOAD(n,sizeof(struct rtgenmsg))
+#endif
+
 /* User defined nlmsg_type which is used mostly for logging netlink
  * messages from dump file */
 #define NLMSG_TSTAMP	15
diff --git a/ip/ipnetns.c b/ip/ipnetns.c
index e4038ea72d71..5a213dcf46cd 100644
--- a/ip/ipnetns.c
+++ b/ip/ipnetns.c
@@ -15,6 +15,8 @@
 #include <unistd.h>
 #include <ctype.h>
 
+#include <linux/net_namespace.h>
+
 #include "utils.h"
 #include "ip_common.h"
 #include "namespace.h"
@@ -23,6 +25,7 @@ static int usage(void)
 {
 	fprintf(stderr, "Usage: ip netns list\n");
 	fprintf(stderr, "       ip netns add NAME\n");
+	fprintf(stderr, "       ip netns set NAME NETNSID\n");
 	fprintf(stderr, "       ip [-all] netns delete [NAME]\n");
 	fprintf(stderr, "       ip netns identify [PID]\n");
 	fprintf(stderr, "       ip netns pids NAME\n");
@@ -31,10 +34,56 @@ static int usage(void)
 	exit(-1);
 }
 
+static int get_netnsid_from_name(const char *name)
+{
+	struct {
+		struct nlmsghdr n;
+		struct rtgenmsg g;
+		char            buf[1024];
+	} req, answer;
+	struct rtattr *tb[NETNSA_MAX + 1];
+	struct rtgenmsg *rthdr;
+	int len, fd;
+
+	memset(&req, 0, sizeof(req));
+	req.n.nlmsg_len = NLMSG_LENGTH(sizeof(struct rtgenmsg));
+	req.n.nlmsg_flags = NLM_F_REQUEST;
+	req.n.nlmsg_type = RTM_GETNSID;
+	req.g.rtgen_family = AF_UNSPEC;
+
+	fd = netns_get_fd(name);
+	if (fd < 0)
+		return fd;
+
+	addattr32(&req.n, 1024, NETNSA_FD, fd);
+	if (rtnl_talk(&rth, &req.n, 0, 0, &answer.n) < 0) {
+		close(fd);
+		return -2;
+	}
+	close(fd);
+
+	/* Validate message and parse attributes */
+	if (answer.n.nlmsg_type == NLMSG_ERROR)
+		return -1;
+
+	rthdr = NLMSG_DATA(&answer.n);
+	len = answer.n.nlmsg_len - NLMSG_SPACE(sizeof(*rthdr));
+	if (len < 0)
+		return -1;
+
+	parse_rtattr(tb, NETNSA_MAX, NETNS_RTA(rthdr), len);
+
+	if (tb[NETNSA_NSID])
+		return rta_getattr_u32(tb[NETNSA_NSID]);
+
+	return -1;
+}
+
 static int netns_list(int argc, char **argv)
 {
 	struct dirent *entry;
 	DIR *dir;
+	int id;
 
 	dir = opendir(NETNS_RUN_DIR);
 	if (!dir)
@@ -45,7 +94,11 @@ static int netns_list(int argc, char **argv)
 			continue;
 		if (strcmp(entry->d_name, "..") == 0)
 			continue;
-		printf("%s\n", entry->d_name);
+		printf("%s", entry->d_name);
+		id = get_netnsid_from_name(entry->d_name);
+		if (id >= 0)
+			printf(" (id: %d)", id);
+		printf("\n");
 	}
 	closedir(dir);
 	return 0;
@@ -375,6 +428,61 @@ out_delete:
 	return -1;
 }
 
+static int set_netnsid_from_name(const char *name, int nsid)
+{
+	struct {
+		struct nlmsghdr n;
+		struct rtgenmsg g;
+		char            buf[1024];
+	} req;
+	int fd, err = 0;
+
+	memset(&req, 0, sizeof(req));
+	req.n.nlmsg_len = NLMSG_LENGTH(sizeof(struct rtgenmsg));
+	req.n.nlmsg_flags = NLM_F_REQUEST;
+	req.n.nlmsg_type = RTM_NEWNSID;
+	req.g.rtgen_family = AF_UNSPEC;
+
+	fd = netns_get_fd(name);
+	if (fd < 0)
+		return fd;
+
+	addattr32(&req.n, 1024, NETNSA_FD, fd);
+	addattr32(&req.n, 1024, NETNSA_NSID, nsid);
+	if (rtnl_talk(&rth, &req.n, 0, 0, NULL) < 0)
+		err = -2;
+
+	close(fd);
+	return err;
+}
+
+static int netns_set(int argc, char **argv)
+{
+	char netns_path[MAXPATHLEN];
+	const char *name;
+	int netns, nsid;
+
+	if (argc < 1) {
+		fprintf(stderr, "No netns name specified\n");
+		return -1;
+	}
+	if (argc < 2) {
+		fprintf(stderr, "No nsid specified\n");
+		return -1;
+	}
+	name = argv[0];
+	nsid = atoi(argv[1]);
+
+	snprintf(netns_path, sizeof(netns_path), "%s/%s", NETNS_RUN_DIR, name);
+	netns = open(netns_path, O_RDONLY | O_CLOEXEC);
+	if (netns < 0) {
+		fprintf(stderr, "Cannot open network namespace \"%s\": %s\n",
+			name, strerror(errno));
+		return -1;
+	}
+
+	return set_netnsid_from_name(name, nsid);
+}
 
 static int netns_monitor(int argc, char **argv)
 {
@@ -430,6 +538,9 @@ int do_netns(int argc, char **argv)
 	if (matches(*argv, "add") == 0)
 		return netns_add(argc-1, argv+1);
 
+	if (matches(*argv, "set") == 0)
+		return netns_set(argc-1, argv+1);
+
 	if (matches(*argv, "delete") == 0)
 		return netns_delete(argc-1, argv+1);
 
diff --git a/man/man8/ip-netns.8 b/man/man8/ip-netns.8
index 8e6999c0abcb..28a9544143e5 100644
--- a/man/man8/ip-netns.8
+++ b/man/man8/ip-netns.8
@@ -24,6 +24,10 @@ ip-netns \- process network namespace management
 .RI "[ " NETNSNAME " ]"
 
 .ti -8
+.BR "ip netns" " { " set " } "
+.I NETNSNAME NETNSID
+
+.ti -8
 .BR "ip netns identify"
 .RI "[ " PID " ]"
 
@@ -93,6 +97,16 @@ If
 option was specified then all the network namespace names will be removed.
 
 .TP
+.B ip netns set NAME NETNSID - assign an id to a peer network namespace
+.sp
+This command assigns a id to a peer network namespace. This id is valid
+only in the current network namespace.
+This id will be used by the kernel in some netlink messages. If no id is
+assigned when the kernel needs it, it will be automatically assigned by
+the kernel.
+Once it is assigned, it's not possible to change it.
+
+.TP
 .B ip netns identify [PID] - Report network namespaces names for process
 .sp
 This command walks through /var/run/netns and finds all the network
-- 
2.2.2

^ permalink raw reply related

* [PATCH v3 iproute2 1/4] include: update headers
From: Nicolas Dichtel @ 2015-02-17 16:30 UTC (permalink / raw)
  To: shemminger; +Cc: netdev, Nicolas Dichtel
In-Reply-To: <54D48308.60509@6wind.com>

Copy from sanitized headers.

Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
v3: rebase on master branch
v2: took last headers from net-next

 include/linux/net_namespace.h | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)
 create mode 100644 include/linux/net_namespace.h

diff --git a/include/linux/net_namespace.h b/include/linux/net_namespace.h
new file mode 100644
index 000000000000..9a92b7e14a19
--- /dev/null
+++ b/include/linux/net_namespace.h
@@ -0,0 +1,23 @@
+/* Copyright (c) 2015 6WIND S.A.
+ * Author: Nicolas Dichtel <nicolas.dichtel@6wind.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ */
+#ifndef _LINUX_NET_NAMESPACE_H_
+#define _LINUX_NET_NAMESPACE_H_
+
+/* Attributes of RTM_NEWNSID/RTM_GETNSID messages */
+enum {
+	NETNSA_NONE,
+#define NETNSA_NSID_NOT_ASSIGNED -1
+	NETNSA_NSID,
+	NETNSA_PID,
+	NETNSA_FD,
+	__NETNSA_MAX,
+};
+
+#define NETNSA_MAX		(__NETNSA_MAX - 1)
+
+#endif /* _LINUX_NET_NAMESPACE_H_ */
-- 
2.2.2

^ permalink raw reply related

* Re: [PATCH v2 1/3] Ethernet packet sniffer: Device tree binding and vendor prefix
From: Stathis Voukelatos @ 2015-02-17 16:22 UTC (permalink / raw)
  To: Mark Rutland
  Cc: linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	netdev@vger.kernel.org, abrestic@chromium.org
In-Reply-To: <20150217145124.GM8994@leverpostej>

Hi Mark,

On 17/02/15 14:51, Mark Rutland wrote:

>> +Matched packet bytes and timestamp values are returned through a
>> +FIFO. Timestamps are provided to the module through an externally
>> +generated Gray-encoded counter.
>
> Does this counter unit need to be enabled (or have any input clocks
> enabled)?
>

Yes it does, that is the purpose of the 'tstamp' entry in the
'clock-names' property below.

>> +
>> +Required properties:
>> +- compatible : must be "linn,eth-sniffer"
>
> Is there not a more precise name for this IP block?

It is generally called 'ethernet packet sniffer', maybe
linn,eth-packet-sniffer would be a more descriptive name?

>
>> +- reg : physical addresses and sizes of registers. Must contain 3 entries:
>> +          - registers memory space
>> +          - TX command string memory
>> +          - RX command string memory
>> +- reg-names : must contain the following 3 entries:
>> +                  "regs", "tx-ram", "rx-ram"
>
> It would be nicer to format this as:
>
> - reg: A list of physical address and size pairs corresponding to each
>         entry in reg-names
>
> - reg-names: must contain:
>    * "regs" for the control registers
>    * "tx-ram" for the TX command string memory
>    * "rx-ram" for the RX command string memory
>
> Which avoids redundancy.
>

Will change formatting as suggested

> The phrase "command string" sounds a bit odd. What are these used for
> exactly?

In these two memory areas we program a sequence of bytes in the
format: [cmd][value][cmd][value] to configure the data patterns that
the sniffer should match for Ethernet TX and RX packets respectively.
Maybe 'command memory' would be clearer?

>
>> +- interrupts : sniffer interrupt specifier
>> +- clocks : specify the system clock for the peripheral and
>> +	   the enable clock for the timestamp counter
>> +- clock-names : must contain the "sys" and "tstamp" entries
>
> Similarly here clocks should just be defined in terms of clock-names.

Will reformat similar to the 'regs' field

>
>> +- fifo-block-words : number of words in one data FIFO entry
>
> I didn't see a data FIFO described. Is that dynamically allocated and
> handed to the sniffer, or does that correspond to one of the memory
> regions above?
>

It is a H/W FIFO internal to the module and accessed through
a register. It is divided in blocks and 'fifo-block-words'
specify the number of words in each block. It is needed by
the driver to make sure it reads an entire block, in order
to clear the 'data available' interrupt.

>> +- tstamp-hz : frequency of the timestamp counter
>> +- tstamp-shift : shift value for the timestamp cyclecounter struct
>
> What exactly is this used for?
>
> Are there any docs?

See: include/linux/clocksource.h
The driver uses a cyclecounter and timecounter to convert raw timestamps
to nanoseconds. 'tstamp-shift' refers to the 'shift' field of the
cyclecounter structure, that can be used to improve the precision of
the conversion

>
>> +- tstamp-bits : width in bits of the timestamp counter
>> +
>> +Example:
>> +
>> +sniffer@1814a000 {
>> +	compatible = "linn,eth-sniffer";
>> +	reg = <0x1814a000 0x100>, <0x1814a400 0x400>,
>> +	      <0x1814a800 0x400>;
>> +	reg-names = "regs", "tx-ram", "rx-ram";
>> +	interrupts = <GIC_SHARED 58 IRQ_TYPE_LEVEL_HIGH>;
>> +	clocks = <&clk_core CLK_AUDIO>,
>> +		 <&cr_periph SYS_CLK_EVENT_TIMER>;
>> +	clock-names = "sys", "tstamp";
>> +	fifo-block-words = <4>;
>> +	tstamp-hz = <52000000>;
>> +	tstamp-shift = <27>;
>> +	tstamp-bits = <30>;
>
> This property wasn't documented.
>
> As mentioned previously, I think the relation between this unit and the
> MAC and/or PHY needs to be explicitly described in the DT.

Do you suggest a field along the lines of:
mac = <&eth_controller_0>;
The driver could check that it exists and is valid but does
not need to make use of it.

>
>> +};
>> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
>> index d443279..891c224 100644
>> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
>> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
>> @@ -90,6 +90,7 @@ lacie	LaCie
>>   lantiq	Lantiq Semiconductor
>>   lenovo	Lenovo Group Ltd.
>>   lg	LG Corporation
>> +linn    Linn Products Ltd.
>
> This addition looks fine to me. For some reason it seems to be padded
> with spaces instead of tabs though; is my mail server corrupting things
> or is that the case in the original patch?

Sorry, it was spaces. Will be fixed
>
> Thanks,
> Mark.
>

Thank you,
Stathis

^ permalink raw reply

* RE: If bridge have no sub-interfaces, it's status may be still with 'RUNNING'
From: Zhu, Yanjun @ 2015-02-17 16:14 UTC (permalink / raw)
  To: Stephen Hemminger, David Ahern; +Cc: netdev, Wu, Kuaikuai, Tao, Yue
In-Reply-To: <20150216115137.5aa3f0cd@uryu.home.lan>

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

Hi, all

We should make it compatible. So I made a patch to turn on carrier on this bridge interface when the last sub interface is removed.

Hi, David

Please make tests with this latest patch. Thanks a lot.

Best Regards!
Zhu Yanjun
________________________________________
From: Stephen Hemminger [stephen@networkplumber.org]
Sent: Monday, February 16, 2015 8:51 AM
To: David Ahern
Cc: Zhu, Yanjun; netdev; Wu, Kuaikuai; Tao, Yue
Subject: Re: If bridge have no sub-interfaces, it's status may be still with 'RUNNING'

Originally bridge code would start with carrier off,
but there were users using bridge as a form of dummy device
and this broke them. Therefore it was decided (like 5 yrs ago),
that bridge should start with carrier on.

[-- Attachment #2: 0001-bridge-turn-on-carrier-after-removing-the-last-sub-i.patch --]
[-- Type: text/plain, Size: 1025 bytes --]

From 0d16f302f23c9f5101f849450abc073045a193de Mon Sep 17 00:00:00 2001
From: Zhu Yanjun <Yanjun.Zhu@windriver.com>
Date: Tue, 17 Feb 2015 23:59:08 +0800
Subject: [PATCH 1/1] bridge: turn on carrier after removing the last sub
 interface

When the last sub interface is removed from the bridge, the bridge
turn off the carrier. Maybe it is not compatible with firstly created
bridge without any sub interface. At that time, the carrier of this
bridge is turned on.

Signed-off-by: Zhu Yanjun <Yanjun.Zhu@windriver.com>
---
 net/bridge/br_if.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index ed307db..f391f8c 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -565,6 +565,12 @@ int br_del_if(struct net_bridge *br, struct net_device *dev)
 
 	netdev_update_features(br->dev);
 
+	/* No sub interface in bridge, turn on carrier */
+	if (list_empty(&br->port_list)) {
+		eth_hw_addr_random(br->dev);
+		netif_carrier_on(br->dev);
+	}
+
 	return 0;
 }
 
-- 
1.9.1


^ permalink raw reply related

* Re: [PATCH next v3 2/6] bonding: implement bond_poll_controller()
From: Mahesh Bandewar @ 2015-02-17 16:09 UTC (permalink / raw)
  To: Jay Vosburgh
  Cc: Andy Gospodarek, Veaceslav Falico, Nikolay Aleksandrov,
	David Miller, Maciej Zenczykowski, netdev, Eric Dumazet
In-Reply-To: <15770.1424011874@nyx>

On Sun, Feb 15, 2015 at 6:51 AM, Jay Vosburgh
<jay.vosburgh@canonical.com> wrote:
> Mahesh Bandewar <maheshb@google.com> wrote:
>
>>On Thu, Feb 12, 2015 at 1:45 PM, Jay Vosburgh <jay.vosburgh@canonical.com>
>>wrote:
> [...]
>>    >+ if (BOND_MODE(bond) == BOND_MODE_8023AD)
>>    >+ if (bond_3ad_get_active_agg_info(bond, &ad_info))
>>    >+ return;
>>    >+
>>    >+ bond_for_each_slave(bond, slave, iter) {
>>    >+ ops = slave->dev->netdev_ops;
>>    >+ if (!bond_slave_is_up(slave) || !ops->ndo_poll_controller)
>>    >+ continue;
>>    >+
>>    >+ if (BOND_MODE(bond) == BOND_MODE_8023AD) {
>>    >+ struct aggregator *agg =
>>    >+ SLAVE_AD_INFO(slave)->port.aggregator;
>>    >+
>>    >+ if (agg && agg->aggregator_identifier !=
>>    >+ ad_info.aggregator_id)
>>    >+ continue;
>>    >+ if (!bond_3ad_port_is_active(slave) || ad_info.ports != 1)
>>    >+ continue;
>>
>>
>>    The above will exclude slaves that are in an aggregator with
>>    more than one member, which is likely to be the usual case. Is that
>>    intentional?
>>
>>
>>
>>The idea is to use all the ports in the aggregator. In a situation where
>>there is only one port but is not active, then *only* use it. So from that
>>perspective this logic needs '&&' instead of '||'.
>
>         If you want to use any port from the active aggregator, then I
> think logic to first find the active agg, then cycle through its ports
> would be better.  This would also eliminate the concern from your other
> reply regarding the following:
>
>> >+static int bond_3ad_port_operational(struct slave *slave)
>> >+{
>> >+      port_t *port = &SLAVE_AD_INFO(slave)->port;
>> >+
>> >+      return bond_slave_can_tx(slave) &&
>> >+             (port->actor_oper_port_state & port->partner_oper.port_state &
>> >+              BOND_3AD_PORT_OPERATIONAL) == BOND_3AD_PORT_OPERATIONAL;
>> >+}
>
>         needing to test the port_state; if the logic cycles through the
> ports of the active aggregator, then it shouldn't need to check the
> state.  Any port in the active aggregator should be able to transmit,
> even if it is in the "no LACP peer" fallback situation (with one
> exception, noted below).
>
>         I think something like:
>
>         first_slave = bond_first_slave_rcu(bond);
>         agg = __get_active_agg(&(SLAVE_AD_INFO(first_slave)->aggregator));
>         for (port = agg->lag_ports; port;
>              port = port->next_port_in_aggregator) {
>                 if (bond_slave_can_tx(port->slave))
>                         /* use this one */
>         }
>
>         would do roughly what you're describing, although it will always
> choose the first available port of the active aggregator, even if there
> are more than one.
>
Thanks Jay for the suggestion. I think selecting one (first available
slave) is not sufficient since the xmit code path will determine
(based on the hash) which slave to select for the xmit. So in
poll_controller we've to prep all the slaves that are capable of xmit.

I'll replace is_port_operational() with slave_can_tx() check and keep
the remaining logic that covers remaining modes. That way we can have
one poll_controller() for all modes.

--mahesh..


>         Generally, slaves that are not up will not remain in the active
> aggregator, but the slave_can_tx test will cover the window between when
> a slave goes carrier down and the 3ad logic removes it from the active
> agg.
>
>         -J
>
> ---
>         -Jay Vosburgh, jay.vosburgh@canonical.com

^ permalink raw reply

* Re: [PATCH] ipv4: ip_check_defrag should correctly check return value of skb_copy_bits
From: Eric Dumazet @ 2015-02-17 15:55 UTC (permalink / raw)
  To: Alexander Drozdov, Johannes Berg
  Cc: David S. Miller, Alexey Kuznetsov, James Morris,
	Hideaki YOSHIFUJI, Patrick McHardy, netdev, linux-kernel
In-Reply-To: <1424169226-30112-1-git-send-email-al.drozdov@gmail.com>

On Tue, 2015-02-17 at 13:33 +0300, Alexander Drozdov wrote:
> skb_copy_bits() returns zero on success and negative value on error,
> so it is needed to invert the condition in ip_check_defrag().
> 
> Fixes: 1bf3751ec90c ("ipv4: ip_check_defrag must not modify skb before unsharing")
> Signed-off-by: Alexander Drozdov <al.drozdov@gmail.com>
> ---
>  net/ipv4/ip_fragment.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/ipv4/ip_fragment.c b/net/ipv4/ip_fragment.c
> index e5b6d0d..2c8d98e 100644
> --- a/net/ipv4/ip_fragment.c
> +++ b/net/ipv4/ip_fragment.c
> @@ -664,7 +664,7 @@ struct sk_buff *ip_check_defrag(struct sk_buff *skb, u32 user)
>  	if (skb->protocol != htons(ETH_P_IP))
>  		return skb;
>  
> -	if (!skb_copy_bits(skb, 0, &iph, sizeof(iph)))
> +	if (skb_copy_bits(skb, 0, &iph, sizeof(iph)) < 0)
>  		return skb;
>  
>  	if (iph.ihl < 5 || iph.version != 4)

Thanks, I wonder how this was not found/fixed earlier.

Acked-by: Eric Dumazet <edumazet@google.com>
Cc: Johannes Berg <johannes.berg@intel.com>

^ permalink raw reply

* Re: getting a list of naked interface names from iproute2
From: Stephen Hemminger @ 2015-02-17 15:23 UTC (permalink / raw)
  To: Matthew Thode; +Cc: netdev
In-Reply-To: <54E28630.4020200@mthode.org>

On Mon, 16 Feb 2015 18:07:12 -0600
Matthew Thode <mthode@mthode.org> wrote:

> iproute2 seems to like to add @ and : info to it's output.  This makes
> parsing it hard for things like neutron (openstack).  I have a patch to
> slightly update it (still missing some things, I'll link it at the end
> if interested).  Is there a better way of getting interface names from
> iproute2 without having to strip out at characters like '@' and ':'?
> 
> changeset in neutron: https://review.openstack.org/#/c/154128/
> link to current code:
> https://github.com/openstack/neutron/blob/master/neutron/agent/linux/ip_lib.py#L110-L131
> 
> I'd appreciate your help, I'd rather not have to go through each link
> type to pull it's delimiter, but if needed I guess it's needed.
> 

Stop screenscraping, and use a real API.
All of the above would work better:
  1. Libc if_nameindex
  2. Legacy ioctl(SIOCGIFCONF)
  3. Netlink  NLMMSG_REQ (RTM_GETLINK)
  4. Sysfs /sys/class/net

^ permalink raw reply

* Re: [E1000-devel] [PATCH 1/3] ixgbe, ixgbevf: Add new mbox API to enable MC promiscuous mode
From: Vlad Yasevich @ 2015-02-17 15:02 UTC (permalink / raw)
  To: Hiroshi Shimamoto, Skidmore, Donald C, Kirsher, Jeffrey T
  Cc: Alexander Duyck, Bjørn Mork,
	e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org,
	Choi, Sy Jong, linux-kernel@vger.kernel.org, David Laight,
	Hayato Momma
In-Reply-To: <7F861DC0615E0C47A872E6F3C5FCDDBD05E322FD@BPXM14GP.gisp.nec.co.jp>

On 02/15/2015 11:54 PM, Hiroshi Shimamoto wrote:
>>>>>>>>>>> Can you please fix up your patches based on my tree:
>>>>>>>>>>> git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/que
>>>>>>>>>>> ue.git
>>>>>>>>>>
>>>>>>>>>> Yes. I haven't noticed your tree.
>>>>>>>>>> Will resend patches against it.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I encountered an issue with your tree, the commit id is below.
>>>>>>>>>
>>>>>>>>> $ git log | head
>>>>>>>>> commit e6f1649780f8f5a87299bf6af04453f93d1e3d5e
>>>>>>>>> Author: Rasmus Villemoes <linux@rasmusvillemoes.dk>
>>>>>>>>> Date:   Fri Jan 23 20:43:14 2015 -0800
>>>>>>>>>
>>>>>>>>>     ethernet: fm10k: Actually drop 4 bits
>>>>>>>>>
>>>>>>>>>     The comment explains the intention, but vid has type u16. Before
>>>> the
>>>>>>>>>     inner shift, it is promoted to int, which has plenty of space for all
>>>>>>>>>     vid's bits, so nothing is dropped. Use a simple mask instead.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I use the kernel from your tree in both host and guest.
>>>>>>>>>
>>>>>>>>> Assign an IPv6 for VF in guest.
>>>>>>>>> # ip -6 addr add 2001:db8::18:1/64 dev ens0
>>>>>>>>>
>>>>>>>>> Send ping packet from other server to the VM.
>>>>>>>>> # ping6  2001:db8::18:1 -I eth0
>>>>>>>>>
>>>>>>>>> The following message was shown.
>>>>>>>>> ixgbevf 0000:00:08.0: partial checksum but l4 proto=3a!
>>>>>>>>>
>>>>>>>>> If I did the same operation in the host, I saw the same error
>>>>>>>>> message in
>>>>>>> host too.
>>>>>>>>> ixgbe 0000:2d:00.0: partial checksum but l4 proto=3a!
>>>>>>>>>
>>>>>>>>> Do you have any idea about that?
>>>>>>>>
>>>>>>>> Ah, sorry about that, try this tree again:
>>>>>>>> git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/queue.git
>>>>>>>>
>>>>>>>> That patch was dropped for favor of a patch that Matthew Vick
>>>>>>>> put together (and recently got pushed upstream).  So my queue no
>>>>>>>> longer has that patch in the queue, since it got dropped.
>>>>>>>
>>>>>>> I still see the same error, the head id is the below
>>>>>>>
>>>>>>> $ git log | head
>>>>>>> commit a072afb0b45904022b76deef3b770ee9a93cb13a
>>>>>>> Author: Nicholas Krause <xerofoify@gmail.com>
>>>>>>> Date:   Mon Feb 9 00:27:00 2015 -0800
>>>>>>>
>>>>>>>     igb: Remove outdated fix me comment in the
>>>>>>> function,gb_acquire_swfw_sync_i210
>>>>>>>
>>>>>>>
>>>>>>> thanks,
>>>>>>> Hiroshi
>>>>>>
>>>>>> I'm having our validation see if they can recreate the same issue
>>>>>> internally.  When they get back to me I'll let you
>>>>> know
>>>>>> what we found.
>>>>>
>>>>> We did bisect, and the below looks the culprit;
>>>>>
>>>>> 32dce968dd987adfb0c00946d78dad9154f64759 is the first bad commit
>>>>> commit 32dce968dd987adfb0c00946d78dad9154f64759
>>>>> Author: Vlad Yasevich <vyasevich@gmail.com>
>>>>> Date:   Sat Jan 31 10:40:18 2015 -0500
>>>>>
>>>>>     ipv6: Allow for partial checksums on non-ufo packets
>>>>>
>>>>>     Currntly, if we are not doing UFO on the packet, all UDP
>>>>>     packets will start with CHECKSUM_NONE and thus perform full
>>>>>     checksum computations in software even if device support
>>>>>     IPv6 checksum offloading.
>>>>>
>>>>>     Let's start start with CHECKSUM_PARTIAL if the device
>>>>>     supports it and we are sending only a single packet at
>>>>>     or below mtu size.
>>>>>
>>>>>     Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
>>>>>     Signed-off-by: David S. Miller <davem@davemloft.net>
>>>>>
>>>>> :040000 040000 4437eaf7e944f5a6136ebf668a256fee688fda3d
>>>> fade8da998d35c8da97a15f0556949ad371e5347 M      net
>>>>
>>>> When I reverted the commit, the issue was solved.
>>>>
>>>> thanks,
>>>> Hiroshi
>>>
>>> I believe the issue is that this patch (32dce968dd98 - ipv6: Allow for partial checksums on non-ufo packets) is that
>> it now sets CHECKSUM_PARTIAL on all IPv6 packets including ICMPv6 ones.  Our HW (82599) only supports checksum offload
>> on TCP/UDP (NETIF_F_IPV6_CSUM) so we get hung up on the skb's protocol and the fact that it is CHECKSUM_PARTIAL.
>>>
>>> Another thing that confuses me is the feature test in this patch.  It checks (rt->dst.dev->features & NETIF_F_V6_CSUM)
>> but NETIF_F_V6_CSUM is a two bit field?
>>>
>>> #define NETIF_F_V6_CSUM         (NETIF_F_GEN_CSUM | NETIF_F_IPV6_CSUM)
>>>
>>> So the test would succeed if either bit was high, that doesn't seem right.  I cc'd the author so maybe he could clue
>> us in.
>>
>> This has been addressed by:
>> commit bf250a1fa769f2eb8fc7a4e28b3b523e9cb67eef
>> Author: Vlad Yasevich <vyasevich@gmail.com>
>> Date:   Tue Feb 10 11:37:29 2015 -0500
>>
>>     ipv6: Partial checksum only UDP packets
>>
>>
>> As far the 2 bit issue, GEN_CSUM (HW_SUM) and IPV6_CSUM can not coexist at the same time.
>> See netdev_fix_features().
>>
> 
> thanks for pointing it. I will test with that commit.
> 
> Jeff's tree hasn't included that commit yet, right?
> Which branch has the commit?

This is in DaveM's net and net-next trees.

-vlad

> 
> thanks,
> Hiroshi
> 

^ permalink raw reply

* Re: [PATCH] vhost: support upto 509 memory regions
From: Igor Mammedov @ 2015-02-17 15:02 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: Paolo Bonzini, linux-kernel, kvm, netdev
In-Reply-To: <20150217132931.GB6362@redhat.com>

On Tue, 17 Feb 2015 14:29:31 +0100
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Tue, Feb 17, 2015 at 02:11:37PM +0100, Paolo Bonzini wrote:
> > 
> > 
> > On 17/02/2015 13:32, Michael S. Tsirkin wrote:
> > > On Tue, Feb 17, 2015 at 11:59:48AM +0100, Paolo Bonzini wrote:
> > >>
> > >>
> > >> On 17/02/2015 10:02, Michael S. Tsirkin wrote:
> > >>>> Increasing VHOST_MEMORY_MAX_NREGIONS from 65 to 509
> > >>>> to match KVM_USER_MEM_SLOTS fixes issue for vhost-net.
> > >>>>
> > >>>> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > >>>
> > >>> This scares me a bit: each region is 32byte, we are talking
> > >>> a 16K allocation that userspace can trigger.
> > >>
> > >> What's bad with a 16K allocation?
> > > 
> > > It fails when memory is fragmented.
> > 
> > If memory is _that_ fragmented I think you have much bigger problems
> > than vhost.
> > 
> > > I'm guessing kvm doesn't do memory scans on data path, vhost does.
> > 
> > It does for MMIO memory-to-memory writes, but that's not a particularly
> > fast path.
> > 
> > KVM doesn't access the memory map on fast paths, but QEMU does, so I
> > don't think it's beyond the expectations of the kernel.
> 
> QEMU has an elaborate data structure to deal with that.
> 
> >  For example you
> > can use a radix tree (not lib/radix-tree.c unfortunately), and cache
> > GVA->HPA translations if it turns out that lookup has become a hot path.
> 
> All vhost lookups are hot path.
> 
> > The addressing space of x86 is in practice 44 bits or fewer, and each
> > slot will typically be at least 1 GiB, so you only have 14 bits to
> > dispatch on.   It's probably possible to only have two or three levels
> > in the radix tree in the common case, and beat the linear scan real quick.
> 
> Not if there are about 6 regions, I think.
When memslots where increased to 509 and look up of them was replaced on
binary search results were on par with linear search for a default 13 memslots VM.

Adding LRU cache helped to shave ~40% of cycles for sequential lookup workloads.

> 
> > The radix tree can be tuned to use order-0 allocations, and then your
> > worries about fragmentation go away too.
> > 
> > Paolo
> 
> Increasing the number might be reasonable for workloads such as nested
> virt. But depending on this in userspace when you don't have to is not a
> good idea IMHO.
> 
> 

^ permalink raw reply

* Re: [PATCH v2 1/3] Ethernet packet sniffer: Device tree binding and vendor prefix
From: Mark Rutland @ 2015-02-17 14:51 UTC (permalink / raw)
  To: Stathis Voukelatos
  Cc: linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	netdev@vger.kernel.org, abrestic@chromium.org
In-Reply-To: <0a86907642a97e5bd880f69299664232fcffaf9d.1424181053.git.stathis.voukelatos@linn.co.uk>

On Tue, Feb 17, 2015 at 02:03:31PM +0000, Stathis Voukelatos wrote:
> Signed-off-by: Stathis Voukelatos <stathis.voukelatos@linn.co.uk>
> ---
>  .../bindings/net/linn-ether-packet-sniffer.txt     | 42 ++++++++++++++++++++++
>  .../devicetree/bindings/vendor-prefixes.txt        |  1 +
>  2 files changed, 43 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/linn-ether-packet-sniffer.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/linn-ether-packet-sniffer.txt b/Documentation/devicetree/bindings/net/linn-ether-packet-sniffer.txt
> new file mode 100644
> index 0000000..74bac5e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/linn-ether-packet-sniffer.txt
> @@ -0,0 +1,42 @@
> +* Linn Products Ethernet Packet Sniffer
> +The module allows Ethernet packets to be parsed, matched against
> +a user-defined pattern and timestamped. It sits between a 100M
> +Ethernet MAC and PHY and is completely passive with respect to
> +Ethernet frames.
> +Matched packet bytes and timestamp values are returned through a
> +FIFO. Timestamps are provided to the module through an externally
> +generated Gray-encoded counter.

Does this counter unit need to be enabled (or have any input clocks
enabled)?

> +
> +Required properties:
> +- compatible : must be "linn,eth-sniffer"

Is there not a more precise name for this IP block?

> +- reg : physical addresses and sizes of registers. Must contain 3 entries:
> +          - registers memory space
> +          - TX command string memory
> +          - RX command string memory
> +- reg-names : must contain the following 3 entries:
> +                  "regs", "tx-ram", "rx-ram"

It would be nicer to format this as:

- reg: A list of physical address and size pairs corresponding to each
       entry in reg-names

- reg-names: must contain:
  * "regs" for the control registers
  * "tx-ram" for the TX command string memory
  * "rx-ram" for the RX command string memory

Which avoids redundancy.

The phrase "command string" sounds a bit odd. What are these used for
exactly?

> +- interrupts : sniffer interrupt specifier
> +- clocks : specify the system clock for the peripheral and
> +	   the enable clock for the timestamp counter
> +- clock-names : must contain the "sys" and "tstamp" entries

Similarly here clocks should just be defined in terms of clock-names.

> +- fifo-block-words : number of words in one data FIFO entry

I didn't see a data FIFO described. Is that dynamically allocated and
handed to the sniffer, or does that correspond to one of the memory
regions above?

> +- tstamp-hz : frequency of the timestamp counter
> +- tstamp-shift : shift value for the timestamp cyclecounter struct

What exactly is this used for?

Are there any docs?

> +- tstamp-bits : width in bits of the timestamp counter
> +
> +Example:
> +
> +sniffer@1814a000 {
> +	compatible = "linn,eth-sniffer";
> +	reg = <0x1814a000 0x100>, <0x1814a400 0x400>,
> +	      <0x1814a800 0x400>;
> +	reg-names = "regs", "tx-ram", "rx-ram";
> +	interrupts = <GIC_SHARED 58 IRQ_TYPE_LEVEL_HIGH>;
> +	clocks = <&clk_core CLK_AUDIO>,
> +		 <&cr_periph SYS_CLK_EVENT_TIMER>;
> +	clock-names = "sys", "tstamp";
> +	fifo-block-words = <4>;
> +	tstamp-hz = <52000000>;
> +	tstamp-shift = <27>;
> +	tstamp-bits = <30>;

This property wasn't documented.

As mentioned previously, I think the relation between this unit and the
MAC and/or PHY needs to be explicitly described in the DT.

> +};
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
> index d443279..891c224 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
> @@ -90,6 +90,7 @@ lacie	LaCie
>  lantiq	Lantiq Semiconductor
>  lenovo	Lenovo Group Ltd.
>  lg	LG Corporation
> +linn    Linn Products Ltd.

This addition looks fine to me. For some reason it seems to be padded
with spaces instead of tabs though; is my mail server corrupting things
or is that the case in the original patch?

Thanks,
Mark.

^ permalink raw reply

* r8169: strange increase in ping latency after suspend/resume
From: Holger Hoffstätte @ 2015-02-17 14:46 UTC (permalink / raw)
  To: netdev


While benchmarking things on my home LAN (wired, 1Gbit) I noticed an
apparent asymmetric network behaviour between two hosts: a flood ping
from a -> b was consistently slower than from b -> a, despite same
kernel (3.18.x), same NIC (even same rev/firmware), same everything, and
even a being the faster machine (8-core i7 desktop vs. low-end 4-core i5).

Despite the fact that there are two cheap switches in the path I couldn't
figure out why one way would be faster than the other. After many iterations
of eliminating possible causes of my tweaked kernel (with/without BFS CPU
scheduler, SMT awareness, offload settings, HZ/NOHZ and whatnot) I found
that the difference is much more subtle: reboot vs. suspend/resume.

Since I suspend/resume the desktop nightly but not the server, the
behaviour would never become visible in the other direction.

From desktop to server (tux aka. 192.168.100.222), hosts & network idle:

$ping -q -c 10000 -i 0 tux

after a reboot:

--- tux ping statistics ---
10000 packets transmitted, 10000 received, 0% packet loss, time 647ms
rtt min/avg/max/mdev = 0.038/0.046/0.152/0.004 ms, ipg/ewma 0.064/0.041 ms

--- tux ping statistics ---
10000 packets transmitted, 10000 received, 0% packet loss, time 645ms
rtt min/avg/max/mdev = 0.041/0.046/0.061/0.003 ms, ipg/ewma 0.064/0.045 ms

--- tux ping statistics ---
10000 packets transmitted, 10000 received, 0% packet loss, time 639ms
rtt min/avg/max/mdev = 0.039/0.045/0.151/0.009 ms, ipg/ewma 0.063/0.046 ms

This is not too bad considering cheap equipment + two switches in the path,
and consistent with measurements in the other direction (server to desktop)
which would also see *very slightly* increase due to the host being slower.
So that's good and consistent.

Now I suspend/resume the desktop, and we get:

--- tux ping statistics ---
10000 packets transmitted, 10000 received, 0% packet loss, time 930ms
rtt min/avg/max/mdev = 0.041/0.050/0.150/0.009 ms, ipg/ewma 0.093/0.053 ms

--- tux ping statistics ---
10000 packets transmitted, 10000 received, 0% packet loss, time 952ms
rtt min/avg/max/mdev = 0.042/0.051/0.132/0.005 ms, ipg/ewma 0.095/0.050 ms

--- tux ping statistics ---
10000 packets transmitted, 10000 received, 0% packet loss, time 938ms
rtt min/avg/max/mdev = 0.040/0.050/0.157/0.009 ms, ipg/ewma 0.093/0.052 ms

This is 100% repeatable: after a reboot it's fast; suspend/resume: slow.

While this is no drama and far from being unusable, something is now stealing
~5µs on average. I *think* that this relates to either the r8169 NIC or driver,
but can only offer inconclusive evidence insofar as an old laptop with e1000e
shows consistently slower performance, regardless of reboot vs. wakeup.

Any ideas or suggestions?

thanks
Holger

^ permalink raw reply

* Re: [PATCH] vhost: support upto 509 memory regions
From: Paolo Bonzini @ 2015-02-17 14:45 UTC (permalink / raw)
  To: Igor Mammedov, Michael S. Tsirkin; +Cc: linux-kernel, kvm, netdev
In-Reply-To: <20150217154452.6f62dd77@nial.brq.redhat.com>



On 17/02/2015 15:44, Igor Mammedov wrote:
> As alternative:
> we can filter out hotplugged memory and vhost will continue to work with
> only initial memory.
> So question is id we have to tell vhost about hotplugged memory?

Yes, I think so.

Paolo

^ permalink raw reply

* Re: [PATCH] vhost: support upto 509 memory regions
From: Igor Mammedov @ 2015-02-17 14:44 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: Paolo Bonzini, linux-kernel, kvm, netdev
In-Reply-To: <20150217123212.GA6362@redhat.com>

On Tue, 17 Feb 2015 13:32:12 +0100
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Tue, Feb 17, 2015 at 11:59:48AM +0100, Paolo Bonzini wrote:
> > 
> > 
> > On 17/02/2015 10:02, Michael S. Tsirkin wrote:
> > > > Increasing VHOST_MEMORY_MAX_NREGIONS from 65 to 509
> > > > to match KVM_USER_MEM_SLOTS fixes issue for vhost-net.
> > > > 
> > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > >
> > > This scares me a bit: each region is 32byte, we are talking
> > > a 16K allocation that userspace can trigger.
> > 
> > What's bad with a 16K allocation?
> 
> It fails when memory is fragmented.
> 
> > > How does kvm handle this issue?
> > 
> > It doesn't.
> > 
> > Paolo
> 
> I'm guessing kvm doesn't do memory scans on data path,
> vhost does.
> 
> qemu is just doing things that kernel didn't expect it to need.
> 
> Instead, I suggest reducing number of GPA<->HVA mappings:
> 
> you have GPA 1,5,7
> map them at HVA 11,15,17
> then you can have 1 slot: 1->11
> 
> To avoid libc reusing the memory holes, reserve them with MAP_NORESERVE
> or something like this.
Lets suppose that we add API to reserve whole memory hotplug region
with MAP_NORESERVE and passed it as memslot to KVM.

Then what will happen to guest accessing not really mapped region?
This memslot will also be passed to vhost as region, is it really ok?
I don't know what else it might break.

As alternative:
we can filter out hotplugged memory and vhost will continue to work with
only initial memory.
So question is id we have to tell vhost about hotplugged memory?

> 
> We can discuss smarter lookup algorithms but I'd rather
> userspace didn't do things that we then have to
> work around in kernel.
> 
> 

^ 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