Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next v6 3/3] openvswitch: enable NSH support
From: Hannes Frederic Sowa @ 2017-09-06 10:02 UTC (permalink / raw)
  To: Jan Scheurich
  Cc: Yang, Yi, netdev@vger.kernel.org, dev@openvswitch.org,
	jbenc@redhat.com, e@erig.me, blp@ovn.org
In-Reply-To: <CFF8EF42F1132E4CBE2BF0AB6C21C58D787F5ECB@ESESSMB107.ericsson.se>

Jan Scheurich <jan.scheurich@ericsson.com> writes:

>> > There is no way we can re-use the existing TLV tunnel metadata
>> > infrastructure in OVS for matching and setting NSH MD2 TLV headers. We
>> > will need to introduce a new (perhaps similar) scheme for modelling
>> > generic TLV match registers in OVS that are assigned to protocol TLVs
>> > by the controller. This is FFS.
>> 
>> This is what I don't understand.
>> 
>> Why can't you just reuse the space in the struct sw_flow_key where
>> geneve would put in their metadata. There are 255 empty bytes at the
>> beginning if you don't have other tunnel metadata anyway.
>> 
>> If you receive packets over vxlan(gpe), tun_opts gets populated with an
>> ip_tunnel_key. Couldn't you use the options space in there after the
>> ip_tunnel_key to store the NSH context just for the sake of storing them
>> somewhere instead of adding 16 bytes to sw_flow_key?
>
> There is a significant conceptual difference between tunnel metadata
> (copied from a popped tunnel header) and packed match fields extracted
> during parsing of the packets. If we'd store them in the same space in
> the sw_flow_key struct, we are calling for trouble.
>
> NSH is transport agnostic, it should work over Ethernet, VXLAN(GPE)
> and other transport tunnels. Think about an NSH packet arriving on an
> Geneve tunnel port. Any Geneve tunnel options have already been stored
> in the tun_opts metadata bytes. Now the datapath parses the NSH header
> and overwrites the tun_opts metadata with the NSH metadata. This would
> break the OVS semantics.

Obviously you would use key->tun_opts_len and start appending there and
not simply overwrite. Otherwise that would be rather silly.

> I absolutely understand your concern about efficient space utilization
> in the flow struct for TLV match fields and it will be part of the
> design challenge for MD2 TLV support to find a good balance between
> memory and run-time efficiency. But that is FFS. For the four fixed
> size MD1 headers the decision has been to include them as additional
> attributes in the flow key.

Okay, then.

Bye,
Hannes

^ permalink raw reply

* Re: [PATCH net-next v6 3/3] openvswitch: enable NSH support
From: Jan Scheurich @ 2017-09-06  9:54 UTC (permalink / raw)
  To: Hannes Frederic Sowa
  Cc: dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	jbenc-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org, e@erig.me
In-Reply-To: <87bmmo9ngt.fsf-tFNcAqjVMyqKXQKiL6tip0B+6BGkLq7r@public.gmane.org>

> > There is no way we can re-use the existing TLV tunnel metadata
> > infrastructure in OVS for matching and setting NSH MD2 TLV headers. We
> > will need to introduce a new (perhaps similar) scheme for modelling
> > generic TLV match registers in OVS that are assigned to protocol TLVs
> > by the controller. This is FFS.
> 
> This is what I don't understand.
> 
> Why can't you just reuse the space in the struct sw_flow_key where
> geneve would put in their metadata. There are 255 empty bytes at the
> beginning if you don't have other tunnel metadata anyway.
> 
> If you receive packets over vxlan(gpe), tun_opts gets populated with an
> ip_tunnel_key. Couldn't you use the options space in there after the
> ip_tunnel_key to store the NSH context just for the sake of storing them
> somewhere instead of adding 16 bytes to sw_flow_key?

There is a significant conceptual difference between tunnel metadata (copied from a popped tunnel header) and packed match fields extracted during parsing of the packets. If we'd store them in the same space in the sw_flow_key struct, we are calling for trouble.

NSH is transport agnostic, it should work over Ethernet, VXLAN(GPE) and other transport tunnels. Think about an NSH packet arriving on an Geneve tunnel port. Any Geneve tunnel options have already been stored in the tun_opts metadata bytes. Now the datapath parses the NSH header and overwrites the tun_opts metadata with the NSH metadata. This would break the OVS semantics.

I absolutely understand your concern about efficient space utilization in the flow struct for TLV match fields and it will be part of the design challenge for MD2 TLV support to find a good balance between memory and run-time efficiency. But that is FFS. For the four fixed size MD1 headers the decision has been to include them as additional attributes in the flow key.

BR, Jan

^ permalink raw reply

* Re: [PATCH v2 rfc 0/8] IGMP snooping for local traffic
From: Nikolay Aleksandrov @ 2017-09-06  9:52 UTC (permalink / raw)
  To: Stephen Hemminger, Andrew Lunn
  Cc: netdev, jiri, Florian Fainelli, Vivien Didelot
In-Reply-To: <20170905171141.7040519b@xeon-e3>

On 06/09/17 03:11, Stephen Hemminger wrote:
> On Wed,  6 Sep 2017 01:35:02 +0200
> Andrew Lunn <andrew@lunn.ch> wrote:
> 
>> After the very useful feedback from Nikolay, i threw away what i had,
>> and started again. To recap:
>>
>> The linux bridge supports IGMP snooping. It will listen to IGMP
>> reports on bridge ports and keep track of which groups have been
>> joined on an interface. It will then forward multicast based on this
>> group membership.
>>
>> When the bridge adds or removed groups from an interface, it uses
>> switchdev to request the hardware add an mdb to a port, so the
>> hardware can perform the selective forwarding between ports.
>>
>> What is not covered by the current bridge code, is IGMP joins/leaves
>> from the host on the brX interface. These are not reported via
>> switchdev so that hardware knows the local host is interested in the
>> multicast frames.
>>
>> Luckily, the bridge does track joins/leaves on the brX interface. The
>> code is obfusticated, which is why i missed it with my first attempt.
>> So the first patch tries to remove this obfustication. Currently,
>> there is no notifications sent when the bridge interface joins a
>> group. The second patch adds them. bridge monitor then shows
>> joins/leaves in the same way as for other ports of the bridge.
>>
>> Then starts the work passing down to the hardware that the host has
>> joined/left a group. The existing switchdev mdb object cannot be used,
>> since the semantics are different. The existing
>> SWITCHDEV_OBJ_ID_PORT_MDB is used to indicate a specific multicast
>> group should be forwarded out that port of the switch. However here we
>> require the exact opposite. We want multicast frames for the group
>> received on the port to the forwarded to the host. Hence add a new
>> object SWITCHDEV_OBJ_ID_HOST_MDB, a multicast database entry to
>> forward to the host. This new object is then propagated through the
>> DSA layers. No DSA driver changes should be needed, this should just
>> work...
>>
>> Getting the frames to the bridge as requested turned up an issue or
>> three. The offload_fwd_mark is not being set by DSA, so the bridge
>> floods the received frames back to the switch ports, resulting in
>> duplication since the hardware has already flooded the packet. Fixing
>> that turned up an issue with the meaning of
>> SWITCHDEV_ATTR_ID_PORT_PARENT_ID in DSA. A DSA fabric of three
>> switches needs to look to the software bridge as a single
>> switch. Otherwise the offload_fwd_mark does not work, and we get
>> duplication on the non-ingress switch. But each switch returned a
>> different value. And they were not unique.
>>
>> The third and last issue will be explained in a followup email.
>>
>> Open questions:
>>
>> Is sending notifications going to break userspace?
>> Is this new switchdev object O.K. for the few non-DSA switches that exist?
>> Is the SWITCHDEV_ATTR_ID_PORT_PARENT_ID change acceptable?
>>
>>    Andrew
>>
>> Andrew Lunn (8):
>>   net: bridge: Rename mglist to host_joined
>>   net: bridge: Send notification when host join/leaves a group
>>   net: bridge: Add/del switchdev object on host join/leave
>>   net: dsa: slave: Handle switchdev host mdb add/del
>>   net: dsa: switch: handle host mdb add/remove
>>   net: dsa: switch: Don't add CPU port to an mdb by default
>>   net: dsa: set offload_fwd_mark on received packets
>>   net: dsa: Fix SWITCHDEV_ATTR_ID_PORT_PARENT_ID
>>
>>  include/net/switchdev.h   |  1 +
>>  net/bridge/br_input.c     |  2 +-
>>  net/bridge/br_mdb.c       | 50 +++++++++++++++++++++++++++++---
>>  net/bridge/br_multicast.c | 18 +++++++-----
>>  net/bridge/br_private.h   |  2 +-
>>  net/dsa/dsa.c             |  1 +
>>  net/dsa/dsa_priv.h        |  7 +++++
>>  net/dsa/port.c            | 26 +++++++++++++++++
>>  net/dsa/slave.c           | 16 ++++++++---
>>  net/dsa/switch.c          | 72 +++++++++++++++++++++++++++++++++++++++--------
>>  net/switchdev/switchdev.c |  2 ++
>>  11 files changed, 168 insertions(+), 29 deletions(-)
>>
> 
> This looks much cleaner. I don't have DSA hardware or infrastructure to look deeper.
> 

+1

This version looks great!

^ permalink raw reply

* Re: [PATCH net-next v6 3/3] openvswitch: enable NSH support
From: Hannes Frederic Sowa @ 2017-09-06  9:37 UTC (permalink / raw)
  To: Jan Scheurich
  Cc: dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	jbenc-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org, e@erig.me
In-Reply-To: <CFF8EF42F1132E4CBE2BF0AB6C21C58D787F5D2E-hqolJogE5njKJFWPz4pdheaU1rCVNFv4@public.gmane.org>

Jan Scheurich <jan.scheurich-IzeFyvvaP7pWk0Htik3J/w@public.gmane.org> writes:

>> >> Yes, I wrote that in my previous mail. I wonder why NSH context metadata
>> >> is not in tun_metadata as well?
>> >
>> > tun_metadata is tunnel metadata, GENEVE needs tunnel port, but NSH is
>> > not so, NSH can't directly use tun_metadata, for MD type 2, we need to a
>> > lot of rework on tun_metadata to make it shared between GENEVE and NSH,
>> > I don't think this can happen in near term. So tun_metadata isn't option
>> > for this now.
>> 
>> Sorry, I couldn't follow you. Why can't you store the context headers in
>> tun_metadata exactly?
>> 
>
> I think we mixing things. Let me try to clarify:
>
> 1. NSH context metadata has end-to-end significance for the SFP. They
> must be part of the NSH header and cannot be transported as tunnel
> metadata, because transport tunnels (e.g. Geneve) only connect pairs
> of SFFs in the path.

No questions asked. I am not talking about a design choice of the
protocol but an implementation detail of the patch.

> So we need OVS to be able to match on and set NSH context header
> fields, also for MD2 TLVs in the future.

So be it.

> 2. OVS today has support for matching on TLV tunnel metadata after
> termination of a Geneve tunnel. This infrastructure is only usable for
> OVS tunnel ports (like Geneve) but not for matching on TLV headers of
> the NSH protocol, which is not modelled as an OVS tunnel port but
> handled in the OpenFlow pipeline (with generic encp/decap actions to
> enter/terminate an NSH SFP). This was a strategic decision by the OVS
> community two years ago.

I am talking about the tun_opts field in the sw_flow_keys structure for
the kernel dp only.

> There is no way we can re-use the existing TLV tunnel metadata
> infrastructure in OVS for matching and setting NSH MD2 TLV headers. We
> will need to introduce a new (perhaps similar) scheme for modelling
> generic TLV match registers in OVS that are assigned to protocol TLVs
> by the controller. This is FFS.

This is what I don't understand.

Why can't you just reuse the space in the struct sw_flow_key where
geneve would put in their metadata. There are 255 empty bytes at the
beginning if you don't have other tunnel metadata anyway.

If you receive packets over vxlan(gpe), tun_opts gets populated with an
ip_tunnel_key. Couldn't you use the options space in there after the
ip_tunnel_key to store the NSH context just for the sake of storing them
somewhere instead of adding 16 bytes to sw_flow_key?

Thanks,
Hannes

^ permalink raw reply

* Re: [PATCH net-next RFC 1/2] tun: enable NAPI for TUN/TAP driver
From: Willem de Bruijn @ 2017-09-06  9:18 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Petar Penkov, Network Development, Eric Dumazet, Mahesh Bandewar,
	Willem de Bruijn, David Miller, ppenkov
In-Reply-To: <20170905155127.0bd08fb0@xeon-e3>

On Wed, Sep 6, 2017 at 12:51 AM, Stephen Hemminger
<stephen@networkplumber.org> wrote:
> On Tue,  5 Sep 2017 15:35:50 -0700
> Petar Penkov <ppenkov@google.com> wrote:
>
>> Changes TUN driver to use napi_gro_receive() upon receiving packets
>> rather than netif_rx_ni(). Adds flag CONFIG_TUN_NAPI that enables
>> these changes and operation is not affected if the flag is disabled.
>> SKBs are constructed upon packet arrival and are queued to be
>> processed later.
>>
>> The new path was evaluated with a benchmark with the following setup:
>> Open two tap devices and a receiver thread that reads in a loop for
>> each device. Start one sender thread and pin all threads to different
>> CPUs. Send 1M minimum UDP packets to each device and measure sending
>> time for each of the sending methods:
>>       napi_gro_receive():     4.90s
>>       netif_rx_ni():          4.90s
>>       netif_receive_skb():    7.20s
>>
>> Signed-off-by: Petar Penkov <ppenkov@google.com>
>> Cc: Eric Dumazet <edumazet@google.com>
>> Cc: Mahesh Bandewar <maheshb@google.com>
>> Cc: Willem de Bruijn <willemb@google.com>
>> Cc: davem@davemloft.net
>> Cc: ppenkov@stanford.edu
>
> Why is this optional? It adds two code paths both of which need
> to be tested.

If the napi_gro_receive path is no more expensive than netif_receive_skb,
as the evaluation indicates, then it is a good candidate to replace that.

The napi_gro_frags path is purely for code coverage. There is no benefit
to applications to treat data copied from userspace as if it consists of raw
pages of data.

^ permalink raw reply

* Re: [PATCH] DSA support for Micrel KSZ8895
From: Maxim Uvarov @ 2017-09-06  9:14 UTC (permalink / raw)
  To: Tristram.Ha
  Cc: Pavel Machek, Woojung.Huh, Nathan Conrad, Vivien Didelot,
	Florian Fainelli, netdev, linux-kernel, Andrew Lunn
In-Reply-To: <93AF473E2DA327428DE3D46B72B1E9FD4111FA48@CHN-SV-EXMX02.mchp-main.com>

2017-08-31 0:32 GMT+03:00  <Tristram.Ha@microchip.com>:
>> On Mon 2017-08-28 16:09:27, Andrew Lunn wrote:
>> > > I may be confused here, but AFAICT:
>> > >
>> > > 1) Yes, it has standard layout when accessed over MDIO.
>> >
>> >
>> > Section 4.8 of the datasheet says:
>> >
>> >     All the registers defined in this section can be also accessed
>> >     via the SPI interface.
>> >
>> > Meaning all PHY registers can be access via the SPI interface. So you
>> > should be able to make a standard Linux MDIO bus driver which performs
>> > SPI reads.
>>
>> As far as I can tell (and their driver confirms) -- yes, all those registers can be
>> accessed over the SPI, they are just shuffled around... hence MDIO
>> emulation code. I copied it from their code (see the copyrights) so no, I don't
>> believe there's nicer solution.
>>
>> Best regards,
>>
>>                                                                       Pavel
>
> Can you hold on your developing work on KSZ8895 driver?  I am afraid your effort may be in vain.  We at Microchip are planning to release DSA drivers for all KSZ switches, starting at KSZ8795, then KSZ8895, and KSZ8863.
>
> The driver files all follow the structures of the current KSZ9477 DSA driver, and the file tag_ksz.c will be updated to handle the tail tag of different chips, which requires including the ksz_priv.h header.  That is required nevertheless to support using the offload_fwd_mark indication.
>
> The KSZ8795 driver will be submitted after Labor Day (9/4) if testing reveals no problem.  The KSZ8895 driver will be submitted right after that.  You should have no problem using the driver right away.
>

Hello Tristram, is there any update for that driver?

Maxim.


> Tristram Ha
> Principal Software Engineer
> Microchip Technology Inc.
>



-- 
Best regards,
Maxim Uvarov

^ permalink raw reply

* [PATCH v13 3/5] net: e100: Replace PCI pool old API
From: Romain Perier @ 2017-09-06  9:08 UTC (permalink / raw)
  To: Dan Williams, Doug Ledford, Sean Hefty, Hal Rosenstock,
	jeffrey.t.kirsher, David S. Miller, stas.yakovlev, Aviad Krawczyk
  Cc: linux-rdma, netdev, linux-kernel, Greg Kroah-Hartman,
	Romain Perier
In-Reply-To: <20170906090857.14846-1-romain.perier@collabora.com>

The PCI pool API is deprecated. This commit replaces the PCI pool old
API by the appropriate function with the DMA pool API.

Signed-off-by: Romain Perier <romain.perier@collabora.com>
Acked-by: Peter Senna Tschudin <peter.senna@collabora.com>
Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Tested-by: Peter Senna Tschudin <peter.senna@collabora.com>
---
 drivers/net/ethernet/intel/e100.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/intel/e100.c b/drivers/net/ethernet/intel/e100.c
index 4d10270ddf8f..d1470d30351c 100644
--- a/drivers/net/ethernet/intel/e100.c
+++ b/drivers/net/ethernet/intel/e100.c
@@ -607,7 +607,7 @@ struct nic {
 	struct mem *mem;
 	dma_addr_t dma_addr;
 
-	struct pci_pool *cbs_pool;
+	struct dma_pool *cbs_pool;
 	dma_addr_t cbs_dma_addr;
 	u8 adaptive_ifs;
 	u8 tx_threshold;
@@ -1892,7 +1892,7 @@ static void e100_clean_cbs(struct nic *nic)
 			nic->cb_to_clean = nic->cb_to_clean->next;
 			nic->cbs_avail++;
 		}
-		pci_pool_free(nic->cbs_pool, nic->cbs, nic->cbs_dma_addr);
+		dma_pool_free(nic->cbs_pool, nic->cbs, nic->cbs_dma_addr);
 		nic->cbs = NULL;
 		nic->cbs_avail = 0;
 	}
@@ -1910,7 +1910,7 @@ static int e100_alloc_cbs(struct nic *nic)
 	nic->cb_to_use = nic->cb_to_send = nic->cb_to_clean = NULL;
 	nic->cbs_avail = 0;
 
-	nic->cbs = pci_pool_alloc(nic->cbs_pool, GFP_KERNEL,
+	nic->cbs = dma_pool_alloc(nic->cbs_pool, GFP_KERNEL,
 				  &nic->cbs_dma_addr);
 	if (!nic->cbs)
 		return -ENOMEM;
@@ -2961,8 +2961,8 @@ static int e100_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		netif_err(nic, probe, nic->netdev, "Cannot register net device, aborting\n");
 		goto err_out_free;
 	}
-	nic->cbs_pool = pci_pool_create(netdev->name,
-			   nic->pdev,
+	nic->cbs_pool = dma_pool_create(netdev->name,
+			   &nic->pdev->dev,
 			   nic->params.cbs.max * sizeof(struct cb),
 			   sizeof(u32),
 			   0);
@@ -3002,7 +3002,7 @@ static void e100_remove(struct pci_dev *pdev)
 		unregister_netdev(netdev);
 		e100_free(nic);
 		pci_iounmap(pdev, nic->csr);
-		pci_pool_destroy(nic->cbs_pool);
+		dma_pool_destroy(nic->cbs_pool);
 		free_netdev(netdev);
 		pci_release_regions(pdev);
 		pci_disable_device(pdev);
-- 
2.11.0

^ permalink raw reply related

* [PATCH v13 5/5] PCI: Remove PCI pool macro functions
From: Romain Perier @ 2017-09-06  9:08 UTC (permalink / raw)
  To: Dan Williams, Doug Ledford, Sean Hefty, Hal Rosenstock,
	jeffrey.t.kirsher, David S. Miller, stas.yakovlev, Aviad Krawczyk
  Cc: linux-rdma, netdev, linux-kernel, Greg Kroah-Hartman,
	Romain Perier
In-Reply-To: <20170906090857.14846-1-romain.perier@collabora.com>

Now that all the drivers use dma pool API, we can remove the macro
functions for PCI pool.

Signed-off-by: Romain Perier <romain.perier@collabora.com>
Reviewed-by: Peter Senna Tschudin <peter.senna@collabora.com>
---
 include/linux/pci.h | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/include/linux/pci.h b/include/linux/pci.h
index f68c58a93dd0..89dfc277a6c6 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1322,15 +1322,6 @@ int pci_set_vga_state(struct pci_dev *pdev, bool decode,
 #include <linux/pci-dma.h>
 #include <linux/dmapool.h>
 
-#define	pci_pool dma_pool
-#define pci_pool_create(name, pdev, size, align, allocation) \
-		dma_pool_create(name, &pdev->dev, size, align, allocation)
-#define	pci_pool_destroy(pool) dma_pool_destroy(pool)
-#define	pci_pool_alloc(pool, flags, handle) dma_pool_alloc(pool, flags, handle)
-#define	pci_pool_zalloc(pool, flags, handle) \
-		dma_pool_zalloc(pool, flags, handle)
-#define	pci_pool_free(pool, vaddr, addr) dma_pool_free(pool, vaddr, addr)
-
 struct msix_entry {
 	u32	vector;	/* kernel uses to write allocated vector */
 	u16	entry;	/* driver uses to specify entry, OS writes */
-- 
2.11.0

^ permalink raw reply related

* [PATCH v13 4/5] hinic: Replace PCI pool old API
From: Romain Perier @ 2017-09-06  9:08 UTC (permalink / raw)
  To: Dan Williams, Doug Ledford, Sean Hefty, Hal Rosenstock,
	jeffrey.t.kirsher-ral2JQCrhuEAvxtiuMwx3w, David S. Miller,
	stas.yakovlev-Re5JQEeQqe8AvxtiuMwx3w, Aviad Krawczyk
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Greg Kroah-Hartman,
	Romain Perier
In-Reply-To: <20170906090857.14846-1-romain.perier-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>

The PCI pool API is deprecated. This commit replaces the PCI pool old
API by the appropriate function with the DMA pool API.

Signed-off-by: Romain Perier <romain.perier-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
---
 drivers/net/ethernet/huawei/hinic/hinic_hw_cmdq.c | 10 +++++-----
 drivers/net/ethernet/huawei/hinic/hinic_hw_cmdq.h |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/huawei/hinic/hinic_hw_cmdq.c b/drivers/net/ethernet/huawei/hinic/hinic_hw_cmdq.c
index 7d95f0866fb0..28a81ac97af5 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_hw_cmdq.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_hw_cmdq.c
@@ -143,7 +143,7 @@ int hinic_alloc_cmdq_buf(struct hinic_cmdqs *cmdqs,
 	struct hinic_hwif *hwif = cmdqs->hwif;
 	struct pci_dev *pdev = hwif->pdev;
 
-	cmdq_buf->buf = pci_pool_alloc(cmdqs->cmdq_buf_pool, GFP_KERNEL,
+	cmdq_buf->buf = dma_pool_alloc(cmdqs->cmdq_buf_pool, GFP_KERNEL,
 				       &cmdq_buf->dma_addr);
 	if (!cmdq_buf->buf) {
 		dev_err(&pdev->dev, "Failed to allocate cmd from the pool\n");
@@ -161,7 +161,7 @@ int hinic_alloc_cmdq_buf(struct hinic_cmdqs *cmdqs,
 void hinic_free_cmdq_buf(struct hinic_cmdqs *cmdqs,
 			 struct hinic_cmdq_buf *cmdq_buf)
 {
-	pci_pool_free(cmdqs->cmdq_buf_pool, cmdq_buf->buf, cmdq_buf->dma_addr);
+	dma_pool_free(cmdqs->cmdq_buf_pool, cmdq_buf->buf, cmdq_buf->dma_addr);
 }
 
 static unsigned int cmdq_wqe_size_from_bdlen(enum bufdesc_len len)
@@ -875,7 +875,7 @@ int hinic_init_cmdqs(struct hinic_cmdqs *cmdqs, struct hinic_hwif *hwif,
 	int err;
 
 	cmdqs->hwif = hwif;
-	cmdqs->cmdq_buf_pool = pci_pool_create("hinic_cmdq", pdev,
+	cmdqs->cmdq_buf_pool = dma_pool_create("hinic_cmdq", &pdev->dev,
 					       HINIC_CMDQ_BUF_SIZE,
 					       HINIC_CMDQ_BUF_SIZE, 0);
 	if (!cmdqs->cmdq_buf_pool)
@@ -916,7 +916,7 @@ int hinic_init_cmdqs(struct hinic_cmdqs *cmdqs, struct hinic_hwif *hwif,
 	devm_kfree(&pdev->dev, cmdqs->saved_wqs);
 
 err_saved_wqs:
-	pci_pool_destroy(cmdqs->cmdq_buf_pool);
+	dma_pool_destroy(cmdqs->cmdq_buf_pool);
 	return err;
 }
 
@@ -942,5 +942,5 @@ void hinic_free_cmdqs(struct hinic_cmdqs *cmdqs)
 
 	devm_kfree(&pdev->dev, cmdqs->saved_wqs);
 
-	pci_pool_destroy(cmdqs->cmdq_buf_pool);
+	dma_pool_destroy(cmdqs->cmdq_buf_pool);
 }
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_hw_cmdq.h b/drivers/net/ethernet/huawei/hinic/hinic_hw_cmdq.h
index b35583400cb6..23f8d39eab68 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_hw_cmdq.h
+++ b/drivers/net/ethernet/huawei/hinic/hinic_hw_cmdq.h
@@ -157,7 +157,7 @@ struct hinic_cmdq {
 struct hinic_cmdqs {
 	struct hinic_hwif       *hwif;
 
-	struct pci_pool         *cmdq_buf_pool;
+	struct dma_pool         *cmdq_buf_pool;
 
 	struct hinic_wq         *saved_wqs;
 
-- 
2.11.0

--
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 related

* [PATCH v13 2/5] dmaengine: pch_dma: Replace PCI pool old API
From: Romain Perier @ 2017-09-06  9:08 UTC (permalink / raw)
  To: Dan Williams, Doug Ledford, Sean Hefty, Hal Rosenstock,
	jeffrey.t.kirsher, David S. Miller, stas.yakovlev, Aviad Krawczyk
  Cc: linux-rdma, netdev, linux-kernel, Greg Kroah-Hartman,
	Romain Perier
In-Reply-To: <20170906090857.14846-1-romain.perier@collabora.com>

The PCI pool API is deprecated. This commit replaces the PCI pool old
API by the appropriate function with the DMA pool API.

Signed-off-by: Romain Perier <romain.perier@collabora.com>
Acked-by: Peter Senna Tschudin <peter.senna@collabora.com>
Tested-by: Peter Senna Tschudin <peter.senna@collabora.com>
---
 drivers/dma/pch_dma.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/dma/pch_dma.c b/drivers/dma/pch_dma.c
index f9028e9d0dfc..afd8f27bda96 100644
--- a/drivers/dma/pch_dma.c
+++ b/drivers/dma/pch_dma.c
@@ -123,7 +123,7 @@ struct pch_dma_chan {
 struct pch_dma {
 	struct dma_device	dma;
 	void __iomem *membase;
-	struct pci_pool		*pool;
+	struct dma_pool		*pool;
 	struct pch_dma_regs	regs;
 	struct pch_dma_desc_regs ch_regs[MAX_CHAN_NR];
 	struct pch_dma_chan	channels[MAX_CHAN_NR];
@@ -437,7 +437,7 @@ static struct pch_dma_desc *pdc_alloc_desc(struct dma_chan *chan, gfp_t flags)
 	struct pch_dma *pd = to_pd(chan->device);
 	dma_addr_t addr;
 
-	desc = pci_pool_zalloc(pd->pool, flags, &addr);
+	desc = dma_pool_zalloc(pd->pool, flags, &addr);
 	if (desc) {
 		INIT_LIST_HEAD(&desc->tx_list);
 		dma_async_tx_descriptor_init(&desc->txd, chan);
@@ -549,7 +549,7 @@ static void pd_free_chan_resources(struct dma_chan *chan)
 	spin_unlock_irq(&pd_chan->lock);
 
 	list_for_each_entry_safe(desc, _d, &tmp_list, desc_node)
-		pci_pool_free(pd->pool, desc, desc->txd.phys);
+		dma_pool_free(pd->pool, desc, desc->txd.phys);
 
 	pdc_enable_irq(chan, 0);
 }
@@ -880,7 +880,7 @@ static int pch_dma_probe(struct pci_dev *pdev,
 		goto err_iounmap;
 	}
 
-	pd->pool = pci_pool_create("pch_dma_desc_pool", pdev,
+	pd->pool = dma_pool_create("pch_dma_desc_pool", &pdev->dev,
 				   sizeof(struct pch_dma_desc), 4, 0);
 	if (!pd->pool) {
 		dev_err(&pdev->dev, "Failed to alloc DMA descriptors\n");
@@ -931,7 +931,7 @@ static int pch_dma_probe(struct pci_dev *pdev,
 	return 0;
 
 err_free_pool:
-	pci_pool_destroy(pd->pool);
+	dma_pool_destroy(pd->pool);
 err_free_irq:
 	free_irq(pdev->irq, pd);
 err_iounmap:
@@ -963,7 +963,7 @@ static void pch_dma_remove(struct pci_dev *pdev)
 			tasklet_kill(&pd_chan->tasklet);
 		}
 
-		pci_pool_destroy(pd->pool);
+		dma_pool_destroy(pd->pool);
 		pci_iounmap(pdev, pd->membase);
 		pci_release_regions(pdev);
 		pci_disable_device(pdev);
-- 
2.11.0

^ permalink raw reply related

* [PATCH v13 1/5] block: DAC960: Replace PCI pool old API
From: Romain Perier @ 2017-09-06  9:08 UTC (permalink / raw)
  To: Dan Williams, Doug Ledford, Sean Hefty, Hal Rosenstock,
	jeffrey.t.kirsher-ral2JQCrhuEAvxtiuMwx3w, David S. Miller,
	stas.yakovlev-Re5JQEeQqe8AvxtiuMwx3w, Aviad Krawczyk
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Greg Kroah-Hartman,
	Romain Perier
In-Reply-To: <20170906090857.14846-1-romain.perier-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>

The PCI pool API is deprecated. This commit replaces the PCI pool old
API by the appropriate function with the DMA pool API.

Signed-off-by: Romain Perier <romain.perier-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
Acked-by: Peter Senna Tschudin <peter.senna-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
Tested-by: Peter Senna Tschudin <peter.senna-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
---
 drivers/block/DAC960.c | 38 ++++++++++++++++++--------------------
 drivers/block/DAC960.h |  4 ++--
 2 files changed, 20 insertions(+), 22 deletions(-)

diff --git a/drivers/block/DAC960.c b/drivers/block/DAC960.c
index 255591ab3716..2a8950ee382c 100644
--- a/drivers/block/DAC960.c
+++ b/drivers/block/DAC960.c
@@ -268,17 +268,17 @@ static bool DAC960_CreateAuxiliaryStructures(DAC960_Controller_T *Controller)
   void *AllocationPointer = NULL;
   void *ScatterGatherCPU = NULL;
   dma_addr_t ScatterGatherDMA;
-  struct pci_pool *ScatterGatherPool;
+  struct dma_pool *ScatterGatherPool;
   void *RequestSenseCPU = NULL;
   dma_addr_t RequestSenseDMA;
-  struct pci_pool *RequestSensePool = NULL;
+  struct dma_pool *RequestSensePool = NULL;
 
   if (Controller->FirmwareType == DAC960_V1_Controller)
     {
       CommandAllocationLength = offsetof(DAC960_Command_T, V1.EndMarker);
       CommandAllocationGroupSize = DAC960_V1_CommandAllocationGroupSize;
-      ScatterGatherPool = pci_pool_create("DAC960_V1_ScatterGather",
-		Controller->PCIDevice,
+      ScatterGatherPool = dma_pool_create("DAC960_V1_ScatterGather",
+		&Controller->PCIDevice->dev,
 	DAC960_V1_ScatterGatherLimit * sizeof(DAC960_V1_ScatterGatherSegment_T),
 	sizeof(DAC960_V1_ScatterGatherSegment_T), 0);
       if (ScatterGatherPool == NULL)
@@ -290,18 +290,18 @@ static bool DAC960_CreateAuxiliaryStructures(DAC960_Controller_T *Controller)
     {
       CommandAllocationLength = offsetof(DAC960_Command_T, V2.EndMarker);
       CommandAllocationGroupSize = DAC960_V2_CommandAllocationGroupSize;
-      ScatterGatherPool = pci_pool_create("DAC960_V2_ScatterGather",
-		Controller->PCIDevice,
+      ScatterGatherPool = dma_pool_create("DAC960_V2_ScatterGather",
+		&Controller->PCIDevice->dev,
 	DAC960_V2_ScatterGatherLimit * sizeof(DAC960_V2_ScatterGatherSegment_T),
 	sizeof(DAC960_V2_ScatterGatherSegment_T), 0);
       if (ScatterGatherPool == NULL)
 	    return DAC960_Failure(Controller,
 			"AUXILIARY STRUCTURE CREATION (SG)");
-      RequestSensePool = pci_pool_create("DAC960_V2_RequestSense",
-		Controller->PCIDevice, sizeof(DAC960_SCSI_RequestSense_T),
+      RequestSensePool = dma_pool_create("DAC960_V2_RequestSense",
+		&Controller->PCIDevice->dev, sizeof(DAC960_SCSI_RequestSense_T),
 		sizeof(int), 0);
       if (RequestSensePool == NULL) {
-	    pci_pool_destroy(ScatterGatherPool);
+	    dma_pool_destroy(ScatterGatherPool);
 	    return DAC960_Failure(Controller,
 			"AUXILIARY STRUCTURE CREATION (SG)");
       }
@@ -335,16 +335,16 @@ static bool DAC960_CreateAuxiliaryStructures(DAC960_Controller_T *Controller)
       Command->Next = Controller->FreeCommands;
       Controller->FreeCommands = Command;
       Controller->Commands[CommandIdentifier-1] = Command;
-      ScatterGatherCPU = pci_pool_alloc(ScatterGatherPool, GFP_ATOMIC,
+      ScatterGatherCPU = dma_pool_alloc(ScatterGatherPool, GFP_ATOMIC,
 							&ScatterGatherDMA);
       if (ScatterGatherCPU == NULL)
 	  return DAC960_Failure(Controller, "AUXILIARY STRUCTURE CREATION");
 
       if (RequestSensePool != NULL) {
-  	  RequestSenseCPU = pci_pool_alloc(RequestSensePool, GFP_ATOMIC,
+  	  RequestSenseCPU = dma_pool_alloc(RequestSensePool, GFP_ATOMIC,
 						&RequestSenseDMA);
   	  if (RequestSenseCPU == NULL) {
-                pci_pool_free(ScatterGatherPool, ScatterGatherCPU,
+                dma_pool_free(ScatterGatherPool, ScatterGatherCPU,
                                 ScatterGatherDMA);
     		return DAC960_Failure(Controller,
 					"AUXILIARY STRUCTURE CREATION");
@@ -379,8 +379,8 @@ static bool DAC960_CreateAuxiliaryStructures(DAC960_Controller_T *Controller)
 static void DAC960_DestroyAuxiliaryStructures(DAC960_Controller_T *Controller)
 {
   int i;
-  struct pci_pool *ScatterGatherPool = Controller->ScatterGatherPool;
-  struct pci_pool *RequestSensePool = NULL;
+  struct dma_pool *ScatterGatherPool = Controller->ScatterGatherPool;
+  struct dma_pool *RequestSensePool = NULL;
   void *ScatterGatherCPU;
   dma_addr_t ScatterGatherDMA;
   void *RequestSenseCPU;
@@ -411,9 +411,9 @@ static void DAC960_DestroyAuxiliaryStructures(DAC960_Controller_T *Controller)
 	  RequestSenseDMA = Command->V2.RequestSenseDMA;
       }
       if (ScatterGatherCPU != NULL)
-          pci_pool_free(ScatterGatherPool, ScatterGatherCPU, ScatterGatherDMA);
+          dma_pool_free(ScatterGatherPool, ScatterGatherCPU, ScatterGatherDMA);
       if (RequestSenseCPU != NULL)
-          pci_pool_free(RequestSensePool, RequestSenseCPU, RequestSenseDMA);
+          dma_pool_free(RequestSensePool, RequestSenseCPU, RequestSenseDMA);
 
       if ((Command->CommandIdentifier
 	   % Controller->CommandAllocationGroupSize) == 1) {
@@ -437,13 +437,11 @@ static void DAC960_DestroyAuxiliaryStructures(DAC960_Controller_T *Controller)
       Controller->CurrentStatusBuffer = NULL;
     }
 
-  if (ScatterGatherPool != NULL)
-  	pci_pool_destroy(ScatterGatherPool);
+  dma_pool_destroy(ScatterGatherPool);
   if (Controller->FirmwareType == DAC960_V1_Controller)
   	return;
 
-  if (RequestSensePool != NULL)
-	pci_pool_destroy(RequestSensePool);
+  dma_pool_destroy(RequestSensePool);
 
   for (i = 0; i < DAC960_MaxLogicalDrives; i++) {
 	kfree(Controller->V2.LogicalDeviceInformation[i]);
diff --git a/drivers/block/DAC960.h b/drivers/block/DAC960.h
index 85fa9bb63759..47d7d698ece2 100644
--- a/drivers/block/DAC960.h
+++ b/drivers/block/DAC960.h
@@ -2316,7 +2316,7 @@ typedef struct DAC960_Controller
   bool SuppressEnclosureMessages;
   struct timer_list MonitoringTimer;
   struct gendisk *disks[DAC960_MaxLogicalDrives];
-  struct pci_pool *ScatterGatherPool;
+  struct dma_pool *ScatterGatherPool;
   DAC960_Command_T *FreeCommands;
   unsigned char *CombinedStatusBuffer;
   unsigned char *CurrentStatusBuffer;
@@ -2429,7 +2429,7 @@ typedef struct DAC960_Controller
       bool NeedDeviceSerialNumberInformation;
       bool StartLogicalDeviceInformationScan;
       bool StartPhysicalDeviceInformationScan;
-      struct pci_pool *RequestSensePool;
+      struct dma_pool *RequestSensePool;
 
       dma_addr_t	FirstCommandMailboxDMA;
       DAC960_V2_CommandMailbox_T *FirstCommandMailbox;
-- 
2.11.0

--
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 related

* [PATCH v13 0/5] Replace PCI pool by DMA pool API
From: Romain Perier @ 2017-09-06  9:08 UTC (permalink / raw)
  To: Dan Williams, Doug Ledford, Sean Hefty, Hal Rosenstock,
	jeffrey.t.kirsher-ral2JQCrhuEAvxtiuMwx3w, David S. Miller,
	stas.yakovlev-Re5JQEeQqe8AvxtiuMwx3w, Aviad Krawczyk
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Greg Kroah-Hartman,
	Romain Perier

The current PCI pool API are simple macro functions direct expanded to
the appropriate dma pool functions. The prototypes are almost the same
and semantically, they are very similar. I propose to use the DMA pool
API directly and get rid of the old API.

This set of patches, replaces the old API by the dma pool API
and remove the defines.

Changes in v13:
- Rebased series onto next-20170906
- Added a new commit for the hinic ethernet driver
- Remove previously merged patches

Changes in v12:
- Rebased series onto next-20170822

Changes in v11:
- Rebased series onto next-20170809
- Removed patches 08-14, these have been merged.

Changes in v10:
- Rebased series onto next-20170706
- I have fixed and improved patch "scsi: megaraid: Replace PCI pool old API"

Changes in v9:
- Rebased series onto next-20170522
- I have fixed and improved the patch for lpfc driver

Changes in v8:
- Rebased series onto next-20170428

Changes in v7:
- Rebased series onto next-20170416
- Added Acked-by, Tested-by and Reviwed-by tags

Changes in v6:
- Fixed an issue reported by kbuild test robot about changes in DAC960
- Removed patches 15/19,16/19,17/19,18/19. They have been merged by Greg
- Added Acked-by Tags

Changes in v5:
- Re-worded the cover letter (remove sentence about checkpatch.pl)
- Rebased series onto next-20170308
- Fix typos in commit message
- Added Acked-by Tags

Changes in v4:
- Rebased series onto next-20170301
- Removed patch 20/20: checks done by checkpath.pl, no longer required.
  Thanks to Peter and Joe for their feedbacks.
- Added Reviewed-by tags

Changes in v3:
- Rebased series onto next-20170224
- Fix checkpath.pl reports for patch 11/20 and patch 12/20
- Remove prefix RFC
Changes in v2:
- Introduced patch 18/20
- Fixed cosmetic changes: spaces before brace, live over 80 characters
- Removed some of the check for NULL pointers before calling dma_pool_destroy
- Improved the regexp in checkpatch for pci_pool, thanks to Joe Perches
- Added Tested-by and Acked-by tags


Romain Perier (5):
  block: DAC960: Replace PCI pool old API
  dmaengine: pch_dma: Replace PCI pool old API
  net: e100: Replace PCI pool old API
  hinic: Replace PCI pool old API
  PCI: Remove PCI pool macro functions

 drivers/block/DAC960.c                            | 38 +++++++++++------------
 drivers/block/DAC960.h                            |  4 +--
 drivers/dma/pch_dma.c                             | 12 +++----
 drivers/net/ethernet/huawei/hinic/hinic_hw_cmdq.c | 10 +++---
 drivers/net/ethernet/huawei/hinic/hinic_hw_cmdq.h |  2 +-
 drivers/net/ethernet/intel/e100.c                 | 12 +++----
 include/linux/pci.h                               |  9 ------
 7 files changed, 38 insertions(+), 49 deletions(-)

-- 
2.11.0

--
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 6/10] ixgbe: Use ARRAY_SIZE macro
From: Thomas Meyer @ 2017-09-06  9:08 UTC (permalink / raw)
  To: David Miller; +Cc: joe, netdev, linux-kernel
In-Reply-To: <20170905.142205.14687531780483930.davem@davemloft.net>

On Tue, Sep 05, 2017 at 02:22:05PM -0700, David Miller wrote:
> From: Joe Perches <joe@perches.com>
> Date: Tue, 05 Sep 2017 13:01:18 -0700
> 
> > On Tue, 2017-09-05 at 21:45 +0200, Thomas Meyer wrote:
> >> On Tue, Sep 05, 2017 at 11:50:44AM -0700, David Miller wrote:
> >> > From: Thomas Meyer <thomas@m3y3r.de>
> >> > Date: Sun, 03 Sep 2017 14:19:31 +0200
> >> > 
> >> > > Use ARRAY_SIZE macro, rather than explicitly coding some variant of it
> >> > > yourself.
> >> > > Found with: find -type f -name "*.c" -o -name "*.h" | xargs perl -p -i -e
> >> > > 's/\bsizeof\s*\(\s*(\w+)\s*\)\s*\ /\s*sizeof\s*\(\s*\1\s*\[\s*0\s*\]\s*\)
> >> > > /ARRAY_SIZE(\1)/g' and manual check/verification.
> >> > > 
> >> > > Signed-off-by: Thomas Meyer <thomas@m3y3r.de>
> >> > 
> >> > This should be submitted to the Intel ethernet driver maintainers.
> >> 
> >> Hi,
> >> 
> >> my script checks the output of get_maintainer scripts and only sends to "open
> >> list" entries.
> >> 
> >> The intel-wired-lan@lists.osuosl.org is moderated, so that's why the patch
> >> wasn't send there.
> >> 
> >> Strangely the lists for nouveau@lists.freedesktop.org and
> >> intel-gvt-dev@lists.freedesktop.org appears as open lists in the MAINTAINERS
> >> file but seems to be also moderated lists... At least I got some reply that my
> >> message awaits approval. Maybe an update to the MAINTAINERS file is missing
> >> here?
> >> 
> >> I may drop above check in my script and send to all mailing lists that
> >> get_maintainer.pl will return.
> > 
> > There's a difference between moderated and subscriber-only
> > entries in MAINTAINERS.
> > 
> > get_maintainers will by default list moderated lists and
> > not show subscriber-only lists unless using the -s switch.
> 
> Furthermore, nothing prevented you from CC:'ing the maintainer,
> Jeff Kirscher.

Hi,

That's the other condition in my script. I only send to the role
"maintainer" from the output of get_maintainer.pl. But Mr Jeff
Kirscher is only listed as supporter...

Anyway I did bounce the email to him.

with kind regards
thomas

^ permalink raw reply

* [PATCH] tipc: remove unnecessary call to dev_net()
From: Kleber Sacilotto de Souza @ 2017-09-06  9:08 UTC (permalink / raw)
  To: netdev; +Cc: Jon Maloy, Ying Xue

The net device is already stored in the 'net' variable, so no need to call
dev_net() again.

Signed-off-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com>
---
 net/tipc/bearer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c
index 89cd061c4468..6d6395e0904e 100644
--- a/net/tipc/bearer.c
+++ b/net/tipc/bearer.c
@@ -661,7 +661,7 @@ static int tipc_l2_device_event(struct notifier_block *nb, unsigned long evt,
 		break;
 	case NETDEV_UNREGISTER:
 	case NETDEV_CHANGENAME:
-		bearer_disable(dev_net(dev), b);
+		bearer_disable(net, b);
 		break;
 	}
 	return NOTIFY_OK;
-- 
2.14.1

^ permalink raw reply related

* Re: [PATCH net-next] net: cpsw: Don't handle SIOC[GS]HWTSTAMP when CPTS is disabled
From: Richard Cochran @ 2017-09-06  8:59 UTC (permalink / raw)
  To: Grygorii Strashko; +Cc: David Miller, stefan.sorensen, netdev, linux-omap
In-Reply-To: <c470bbf2-fadf-88ba-f944-f0b373d211ad@ti.com>

On Tue, Sep 05, 2017 at 04:25:22PM -0500, Grygorii Strashko wrote:
> I'd like to clarify one thing here - what is the preferable time-stamping
> device: PHY over MAC, or MAC over PHY? 
> my understanding it's PHY and ethtool_get_ts_info() seems already implemented this way.

We simply do not have a way to support MAC and PHY time stamping and
PTP Hardware Clocks at the same time.  Sure, it must be somehow
possible, but that would involve extending SO_TIMESTAMPING yet again,
and this is not worth the effort, IMHO.

There is exactly one PHY on the market that supports time stamping,
and yes, people who design their boards with it generally want to use
it.  They have to enable CONFIG_NETWORK_PHY_TIMESTAMPING and disable
MAC time stamping.

Thanks,
Richard

^ permalink raw reply

* Re: [PATCH 3/6] hid: make device_attribute const
From: Jiri Kosina @ 2017-09-06  8:57 UTC (permalink / raw)
  To: Bhumika Goyal
  Cc: julia.lawall, rjw, lenb, jbacik, benjamin.tissoires,
	manish.chopra, rahul.verma, Dept-GELinuxNICDev, harish.patil,
	cascardo, don, dvhart, andy, sre, linux-acpi, linux-kernel,
	linux-block, nbd-general, linux-input, netdev,
	platform-driver-x86, linux-pm
In-Reply-To: <1503315792-14837-4-git-send-email-bhumirks@gmail.com>

On Mon, 21 Aug 2017, Bhumika Goyal wrote:

> Make this const as it is only passed as an argument to the
> function device_create_file and device_remove_file and the corresponding
> arguments are of type const.
> Done using Coccinelle
> 
> Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
> ---
>  drivers/hid/hid-core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 9bc9116..24e929c 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -1662,7 +1662,7 @@ static bool hid_hiddev(struct hid_device *hdev)
>  	.size = HID_MAX_DESCRIPTOR_SIZE,
>  };
>  
> -static struct device_attribute dev_attr_country = {
> +static const struct device_attribute dev_attr_country = {
>  	.attr = { .name = "country", .mode = 0444 },
>  	.show = show_country,

Applied, thanks.

-- 
Jiri Kosina
SUSE Labs


^ permalink raw reply

* RE: hns3 use of pci_enable_msix_range
From: Salil Mehta @ 2017-09-06  8:43 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: huangdaode, lipeng (Y), Zhuangyuzeng (Yisen), Huwei (Xavier),
	netdev@vger.kernel.org
In-Reply-To: <20170901082633.GA23568@lst.de>

Hi Christoph,
Thanks for bringing this into notice. I will change and send this patch
in the next cycle after we have tested it internally. 

Best regards
Salil
> -----Original Message-----
> From: Christoph Hellwig [mailto:hch@lst.de]
> Sent: Friday, September 01, 2017 9:27 AM
> To: Salil Mehta
> Cc: huangdaode; lipeng (Y); Zhuangyuzeng (Yisen); Huwei (Xavier);
> netdev@vger.kernel.org
> Subject: hns3 use of pci_enable_msix_range
> 
> Hi Salil,
> 
> can you please switch your new hns3 driver to use pci_alloc_irq_vectors
> instead of the deprecated pci_enable_msix_range interface?  Take a
> look at Documentation/PCI/MSI-HOWTO.txt for the details.

^ permalink raw reply

* Re: [PATCH net-next v6 3/3] openvswitch: enable NSH support
From: Jan Scheurich @ 2017-09-06  8:27 UTC (permalink / raw)
  To: Hannes Frederic Sowa, Yang, Yi
  Cc: dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, e@erig.me,
	jbenc-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
In-Reply-To: <87o9qo9ru6.fsf-tFNcAqjVMyqKXQKiL6tip0B+6BGkLq7r@public.gmane.org>

> >> Yes, I wrote that in my previous mail. I wonder why NSH context metadata
> >> is not in tun_metadata as well?
> >
> > tun_metadata is tunnel metadata, GENEVE needs tunnel port, but NSH is
> > not so, NSH can't directly use tun_metadata, for MD type 2, we need to a
> > lot of rework on tun_metadata to make it shared between GENEVE and NSH,
> > I don't think this can happen in near term. So tun_metadata isn't option
> > for this now.
> 
> Sorry, I couldn't follow you. Why can't you store the context headers in
> tun_metadata exactly?
> 

I think we mixing things. Let me try to clarify:

1. NSH context metadata has end-to-end significance for the SFP. They must be part of the NSH header and cannot be transported as tunnel metadata, because transport tunnels (e.g. Geneve) only connect pairs of SFFs in the path. So we need OVS to be able to match on and set NSH context header fields, also for MD2 TLVs in the future. 

2. OVS today has support for matching on TLV tunnel metadata after termination of a Geneve tunnel. This infrastructure is only usable for OVS tunnel ports (like Geneve) but not for matching on TLV headers of the NSH protocol, which is not modelled as an OVS tunnel port but handled in the OpenFlow pipeline (with generic encp/decap actions to enter/terminate an NSH SFP). This was a strategic decision by the OVS community two years ago.

There is no way we can re-use the existing TLV tunnel metadata infrastructure in OVS for matching and setting NSH MD2 TLV headers. We will need to introduce a new (perhaps similar) scheme for modelling generic TLV match registers in OVS that are assigned to protocol TLVs by the controller. This is FFS.

BR, Jan

^ permalink raw reply

* [PATCH net 2/2] i40e: Avoid some useless variables and initializers in nvm functions
From: Stefano Brivio @ 2017-09-06  8:11 UTC (permalink / raw)
  To: Jeff Kirsher, netdev, intel-wired-lan
  Cc: David S . Miller, Anjali Singhai Jain
In-Reply-To: <cover.1504684488.git.sbrivio@redhat.com>

Fixes: 09f79fd49d94 ("i40e: avoid NVM acquire deadlock during NVM update")
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 drivers/net/ethernet/intel/i40e/i40e_nvm.c | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_nvm.c b/drivers/net/ethernet/intel/i40e/i40e_nvm.c
index 200779d5c124..4a8a81cd3e32 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_nvm.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_nvm.c
@@ -294,13 +294,10 @@ static i40e_status i40e_read_nvm_word_aq(struct i40e_hw *hw, u16 offset,
 static i40e_status __i40e_read_nvm_word(struct i40e_hw *hw,
 					u16 offset, u16 *data)
 {
-	i40e_status ret_code = 0;
-
 	if (hw->flags & I40E_HW_FLAG_AQ_SRCTL_ACCESS_ENABLE)
-		ret_code = i40e_read_nvm_word_aq(hw, offset, data);
-	else
-		ret_code = i40e_read_nvm_word_srctl(hw, offset, data);
-	return ret_code;
+		return i40e_read_nvm_word_aq(hw, offset, data);
+
+	return i40e_read_nvm_word_srctl(hw, offset, data);
 }
 
 /**
@@ -314,7 +311,7 @@ static i40e_status __i40e_read_nvm_word(struct i40e_hw *hw,
 i40e_status i40e_read_nvm_word(struct i40e_hw *hw, u16 offset,
 			       u16 *data)
 {
-	i40e_status ret_code = 0;
+	i40e_status ret_code;
 
 	ret_code = i40e_acquire_nvm(hw, I40E_RESOURCE_READ);
 	if (ret_code)
@@ -429,13 +426,10 @@ static i40e_status __i40e_read_nvm_buffer(struct i40e_hw *hw,
 					  u16 offset, u16 *words,
 					  u16 *data)
 {
-	i40e_status ret_code = 0;
-
 	if (hw->flags & I40E_HW_FLAG_AQ_SRCTL_ACCESS_ENABLE)
-		ret_code = i40e_read_nvm_buffer_aq(hw, offset, words, data);
-	else
-		ret_code = i40e_read_nvm_buffer_srctl(hw, offset, words, data);
-	return ret_code;
+		return i40e_read_nvm_buffer_aq(hw, offset, words, data);
+
+	return i40e_read_nvm_buffer_srctl(hw, offset, words, data);
 }
 
 /**
-- 
2.9.4

^ permalink raw reply related

* [PATCH net 1/2] i40e: Fix comment about locking for __i40e_read_nvm_word()
From: Stefano Brivio @ 2017-09-06  8:11 UTC (permalink / raw)
  To: Jeff Kirsher, netdev, intel-wired-lan
  Cc: David S . Miller, Anjali Singhai Jain
In-Reply-To: <cover.1504684488.git.sbrivio@redhat.com>

Caller needs to acquire the lock. Called functions will not.

Fixes: 09f79fd49d94 ("i40e: avoid NVM acquire deadlock during NVM update")
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 drivers/net/ethernet/intel/i40e/i40e_nvm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_nvm.c b/drivers/net/ethernet/intel/i40e/i40e_nvm.c
index 26d7e9fe6220..200779d5c124 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_nvm.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_nvm.c
@@ -281,7 +281,7 @@ static i40e_status i40e_read_nvm_word_aq(struct i40e_hw *hw, u16 offset,
 }
 
 /**
- * __i40e_read_nvm_word - Reads nvm word, assumes called does the locking
+ * __i40e_read_nvm_word - Reads nvm word, assumes caller does the locking
  * @hw: pointer to the HW structure
  * @offset: offset of the Shadow RAM word to read (0x000000 - 0x001FFF)
  * @data: word read from the Shadow RAM
-- 
2.9.4

^ permalink raw reply related

* [PATCH net 0/2] i40e: Two trivial fixes for 09f79fd49d94
From: Stefano Brivio @ 2017-09-06  8:11 UTC (permalink / raw)
  To: Jeff Kirsher, netdev, intel-wired-lan
  Cc: David S . Miller, Anjali Singhai Jain

Two trivial fixes for recently introduced 09f79fd49d94 ("i40e: avoid NVM
acquire deadlock during NVM update"): 1/2 fixes a comment about locking
and 2/2 gets rid of two useless variables and one initializer in nvm
functions.

Stefano Brivio (2):
  i40e: Fix comment about locking for __i40e_read_nvm_word()
  i40e: Avoid some useless variables and initializers in nvm functions

 drivers/net/ethernet/intel/i40e/i40e_nvm.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

-- 
2.9.4

^ permalink raw reply

* Re: [PATCH net-next v6 3/3] openvswitch: enable NSH support
From: Hannes Frederic Sowa @ 2017-09-06  8:03 UTC (permalink / raw)
  To: Yang, Yi
  Cc: dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	jbenc-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org, e@erig.me
In-Reply-To: <20170906005359.GA103260-re2EX8HDrk21gSHoDXDV2kEOCMrvLtNR@public.gmane.org>

"Yang, Yi" <yi.y.yang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> writes:

> On Tue, Sep 05, 2017 at 09:12:09PM +0800, Hannes Frederic Sowa wrote:
>> "Yang, Yi" <yi.y.yang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> writes:
>> 
>> > We can change this later if we really find a better way to handle this
>> > because it isn't defined in include/uapi/linux/openvswitch.h, so I still
>> > have backdoor to do this if needed :-)
>> 
>> Sorry, I can't follow you.
>> 
>> It doesn't matter if something is defined in uapi headers, the
>> observable behavior matters. If you allow users to configure flows with
>> specific fields, it should not stop working at a future point in time.
>
> Anyway this can be changed if it is really needed, so far current way is
> the best one we can come up with, we would like to use it if you can
> have better proposal. We have explained repeatedly context headers must
> be matched and set, this is bottom line.

I understand that in the way you use those context fields you will have
to match on those. I understand that there is no way around that.

>> > For our sfc use case in Opendaylight, we use context[0] for tunnel ID,
>> > context[1] for destination IP for reverse RSP, they are used to match
>> > and set in OpenFlow table, you can't limit users to use them in such
>> > ways.
>> 
>> So in your specific case you expect the masks to be completely stable
>> because you defined a protocol on top of NSH, understood. And that is
>> stable accross all possible paths. Understood as well.
>> 
>> > If you check GENEVE implementation, tun_metadata* can be set or matched
>> > as any other match field.
>> 
>> Yes, I wrote that in my previous mail. I wonder why NSH context metadata
>> is not in tun_metadata as well?
>
> tun_metadata is tunnel metadata, GENEVE needs tunnel port, but NSH is
> not so, NSH can't directly use tun_metadata, for MD type 2, we need to a
> lot of rework on tun_metadata to make it shared between GENEVE and NSH,
> I don't think this can happen in near term. So tun_metadata isn't option
> for this now.

Sorry, I couldn't follow you. Why can't you store the context headers in
tun_metadata exactly?

[...]

Bye,
Hannes

^ permalink raw reply

* Re: [Patch net] net_sched: fix a memory leak of filter chain
From: Jiri Pirko @ 2017-09-06  7:38 UTC (permalink / raw)
  To: Cong Wang; +Cc: netdev, jakub.kicinski, Jiri Pirko
In-Reply-To: <20170906050310.26474-1-xiyou.wangcong@gmail.com>

Wed, Sep 06, 2017 at 07:03:10AM CEST, xiyou.wangcong@gmail.com wrote:
>tcf_chain_destroy() is called by tcf_block_put() and tcf_chain_put().
>tcf_chain_put() is refcn'ed and paired with tcf_chain_get(),
>but tcf_block_put() is not, it should be paired with tcf_block_get()
>and we still need to decrease the refcnt. However, tcf_block_put()
>is special, it stores the chains too, we have to detach them if
>it is not the last user.

You don't describe the original issue, or I am missing that from your
description.


>
>What's more, index 0 is not special at all, it should be treated
>like other chains. This also makes the code more readable.

[...]


>@@ -246,10 +246,7 @@ EXPORT_SYMBOL(tcf_chain_get);
> 
> void tcf_chain_put(struct tcf_chain *chain)
> {
>-	/* Destroy unused chain, with exception of chain 0, which is the
>-	 * default one and has to be always present.
>-	 */
>-	if (--chain->refcnt == 0 && !chain->filter_chain && chain->index != 0)
>+	if (--chain->refcnt == 0)

The refcounting is only done for actions holding reference to the chain.
You still need to check is the filter chain is not empty.
See tc_ctl_tfilter.

Also, chain 0 is created by default on a block creation. It has to be
present always for a reason. Please see tcf_block_get. The pointer to
chain 0 is assigned to the qdisc filter list pointer.



> 		tcf_chain_destroy(chain);
> }
> EXPORT_SYMBOL(tcf_chain_put);
>@@ -296,8 +293,11 @@ void tcf_block_put(struct tcf_block *block)
> 
> 	list_for_each_entry_safe(chain, tmp, &block->chain_list, list) {
> 		tcf_chain_flush(chain);
>-		tcf_chain_destroy(chain);
>+		tcf_chain_put(chain);
> 	}
>+	/* If tc actions still hold the chain, just detach it. */
>+	list_for_each_entry_safe(chain, tmp, &block->chain_list, list)
>+		tcf_chain_detach(chain);
> 	kfree(block);
> }
> EXPORT_SYMBOL(tcf_block_put);
>-- 
>2.13.0
>

^ permalink raw reply

* Re: [RFC net-next] net: sch_clsact: add support for global per-netns classifier mode
From: Jiri Pirko @ 2017-09-06  7:24 UTC (permalink / raw)
  To: Roopa Prabhu
  Cc: Daniel Borkmann, Cong Wang, Nikolay Aleksandrov,
	Linux Kernel Network Developers, David Ahern, Jamal Hadi Salim
In-Reply-To: <CAJieiUjPvte25P0VOdzYgB34TaLN_veTq3PhRc9yse7REmJaCw@mail.gmail.com>

Wed, Sep 06, 2017 at 06:04:17AM CEST, roopa@cumulusnetworks.com wrote:
>On Tue, Sep 5, 2017 at 3:45 PM, Daniel Borkmann <daniel@iogearbox.net> wrote:
>> On 09/06/2017 12:01 AM, Roopa Prabhu wrote:
>>>
>>> On Tue, Sep 5, 2017 at 11:18 AM, Cong Wang <xiyou.wangcong@gmail.com>
>>> wrote:
>>>>
>>>> On Tue, Sep 5, 2017 at 5:48 AM, Nikolay Aleksandrov
>>>> <nikolay@cumulusnetworks.com> wrote:
>>>>>
>>>>> Hi all,
>>>>> This RFC adds a new mode for clsact which designates a device's egress
>>>>> classifier as global per netns. The packets that are not classified for
>>>>> a particular device will be classified using the global classifier.
>>>>> We have needed a global classifier for some time now for various
>>>>> purposes and setting the single bridge or loopback/vrf device as the
>>
>>
>> Can you elaborate a bit more on the ... "we have needed a global
>> classifier for some time now for various purposes".
>
>Most of our acl's are global or use a wildcard. eg iptables supports
>global rules without an dev. We do end up having hundreds of netdevs.
>Another use case for the future is use of tc for policy based routing
>which requires global rules.

That is not how TC works. There are devices, qdiscs, blocks, chains. The
global approach does not fit. The block sharing gets you what you need,
without need for any ugly hack.

^ permalink raw reply

* Re: [Intel-wired-lan] [PATCH net-next v3] e1000e: Be drop monitor friendly
From: Neftin, Sasha @ 2017-09-06  7:06 UTC (permalink / raw)
  To: Florian Fainelli, netdev
  Cc: edumazet, open list, moderated list:INTEL ETHERNET DRIVERS, davem
In-Reply-To: <20170826011424.27251-1-f.fainelli@gmail.com>

On 8/26/2017 04:14, Florian Fainelli wrote:
> e1000e_put_txbuf() can be called from normal reclamation path as well as
> when a DMA mapping failure, so we need to differentiate these two cases
> when freeing SKBs to be drop monitor friendly. e1000e_tx_hwtstamp_work()
> and e1000_remove() are processing TX timestamped SKBs and those should
> not be accounted as drops either.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> Changes in v3:
>
> - differentiate normal reclamation from TX DMA fragment mapping errors
> - removed a few invalid dev_kfree_skb() replacements (those are already
>    drop monitor friendly)
>
> Changes in v2:
>
> - make it compile
>
>   drivers/net/ethernet/intel/e1000e/netdev.c | 18 +++++++++++-------
>   1 file changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
> index 327dfe5bedc0..cfd21858c095 100644
> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> @@ -1071,7 +1071,8 @@ static bool e1000_clean_rx_irq(struct e1000_ring *rx_ring, int *work_done,
>   }
>   
>   static void e1000_put_txbuf(struct e1000_ring *tx_ring,
> -			    struct e1000_buffer *buffer_info)
> +			    struct e1000_buffer *buffer_info,
> +			    bool drop)
>   {
>   	struct e1000_adapter *adapter = tx_ring->adapter;
>   
> @@ -1085,7 +1086,10 @@ static void e1000_put_txbuf(struct e1000_ring *tx_ring,
>   		buffer_info->dma = 0;
>   	}
>   	if (buffer_info->skb) {
> -		dev_kfree_skb_any(buffer_info->skb);
> +		if (drop)
> +			dev_kfree_skb_any(buffer_info->skb);
> +		else
> +			dev_consume_skb_any(buffer_info->skb);
>   		buffer_info->skb = NULL;
>   	}
>   	buffer_info->time_stamp = 0;
> @@ -1199,7 +1203,7 @@ static void e1000e_tx_hwtstamp_work(struct work_struct *work)
>   		wmb(); /* force write prior to skb_tstamp_tx */
>   
>   		skb_tstamp_tx(skb, &shhwtstamps);
> -		dev_kfree_skb_any(skb);
> +		dev_consume_skb_any(skb);
>   	} else if (time_after(jiffies, adapter->tx_hwtstamp_start
>   			      + adapter->tx_timeout_factor * HZ)) {
>   		dev_kfree_skb_any(adapter->tx_hwtstamp_skb);
> @@ -1254,7 +1258,7 @@ static bool e1000_clean_tx_irq(struct e1000_ring *tx_ring)
>   				}
>   			}
>   
> -			e1000_put_txbuf(tx_ring, buffer_info);
> +			e1000_put_txbuf(tx_ring, buffer_info, false);
>   			tx_desc->upper.data = 0;
>   
>   			i++;
> @@ -2421,7 +2425,7 @@ static void e1000_clean_tx_ring(struct e1000_ring *tx_ring)
>   
>   	for (i = 0; i < tx_ring->count; i++) {
>   		buffer_info = &tx_ring->buffer_info[i];
> -		e1000_put_txbuf(tx_ring, buffer_info);
> +		e1000_put_txbuf(tx_ring, buffer_info, false);
>   	}
>   
>   	netdev_reset_queue(adapter->netdev);
> @@ -5614,7 +5618,7 @@ static int e1000_tx_map(struct e1000_ring *tx_ring, struct sk_buff *skb,
>   			i += tx_ring->count;
>   		i--;
>   		buffer_info = &tx_ring->buffer_info[i];
> -		e1000_put_txbuf(tx_ring, buffer_info);
> +		e1000_put_txbuf(tx_ring, buffer_info, true);
>   	}
>   
>   	return 0;
> @@ -7411,7 +7415,7 @@ static void e1000_remove(struct pci_dev *pdev)
>   	if (adapter->flags & FLAG_HAS_HW_TIMESTAMP) {
>   		cancel_work_sync(&adapter->tx_hwtstamp_work);
>   		if (adapter->tx_hwtstamp_skb) {
> -			dev_kfree_skb_any(adapter->tx_hwtstamp_skb);
> +			dev_consume_skb_any(adapter->tx_hwtstamp_skb);
>   			adapter->tx_hwtstamp_skb = NULL;
>   		}
>   	}

i am ok with this patch

^ 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