* [PATCH 3/3] PCI/msi: remove pci_enable_msi_{exact,range}
From: Christoph Hellwig @ 2017-01-09 20:37 UTC (permalink / raw)
To: linux-pci; +Cc: Mauro Carvalho Chehab, netdev, linux-media
In-Reply-To: <1483994260-19797-1-git-send-email-hch@lst.de>
All multi-MSI allocations are now done through pci_irq_alloc_vectors,
so remove the old interface.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
Documentation/PCI/MSI-HOWTO.txt | 6 ++----
drivers/pci/msi.c | 26 +++++++++-----------------
include/linux/pci.h | 16 ++--------------
3 files changed, 13 insertions(+), 35 deletions(-)
diff --git a/Documentation/PCI/MSI-HOWTO.txt b/Documentation/PCI/MSI-HOWTO.txt
index cd9c9f6..b570a92 100644
--- a/Documentation/PCI/MSI-HOWTO.txt
+++ b/Documentation/PCI/MSI-HOWTO.txt
@@ -162,8 +162,6 @@ The following old APIs to enable and disable MSI or MSI-X interrupts should
not be used in new code:
pci_enable_msi() /* deprecated */
- pci_enable_msi_range() /* deprecated */
- pci_enable_msi_exact() /* deprecated */
pci_disable_msi() /* deprecated */
pci_enable_msix_range() /* deprecated */
pci_enable_msix_exact() /* deprecated */
@@ -268,5 +266,5 @@ or disabled (0). If 0 is found in any of the msi_bus files belonging
to bridges between the PCI root and the device, MSIs are disabled.
It is also worth checking the device driver to see whether it supports MSIs.
-For example, it may contain calls to pci_enable_msi_range() or
-pci_enable_msix_range().
+For example, it may contain calls to pci_irq_alloc_vectors with the
+PCI_IRQ_MSI or PCI_IRQ_MSIX flags.
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 50c5003..16dda43 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -1109,23 +1109,15 @@ static int __pci_enable_msi_range(struct pci_dev *dev, int minvec, int maxvec,
}
}
-/**
- * pci_enable_msi_range - configure device's MSI capability structure
- * @dev: device to configure
- * @minvec: minimal number of interrupts to configure
- * @maxvec: maximum number of interrupts to configure
- *
- * This function tries to allocate a maximum possible number of interrupts in a
- * range between @minvec and @maxvec. It returns a negative errno if an error
- * occurs. If it succeeds, it returns the actual number of interrupts allocated
- * and updates the @dev's irq member to the lowest new interrupt number;
- * the other interrupt numbers allocated to this device are consecutive.
- **/
-int pci_enable_msi_range(struct pci_dev *dev, int minvec, int maxvec)
+/* deprecated, don't use */
+int pci_enable_msi(struct pci_dev *dev)
{
- return __pci_enable_msi_range(dev, minvec, maxvec, NULL);
+ int rc = __pci_enable_msi_range(dev, 1, 1, NULL);
+ if (rc < 0)
+ return rc;
+ return 0;
}
-EXPORT_SYMBOL(pci_enable_msi_range);
+EXPORT_SYMBOL(pci_enable_msi);
static int __pci_enable_msix_range(struct pci_dev *dev,
struct msix_entry *entries, int minvec,
@@ -1381,7 +1373,7 @@ int pci_msi_domain_check_cap(struct irq_domain *domain,
{
struct msi_desc *desc = first_pci_msi_entry(to_pci_dev(dev));
- /* Special handling to support pci_enable_msi_range() */
+ /* Special handling to support __pci_enable_msi_range() */
if (pci_msi_desc_is_multi_msi(desc) &&
!(info->flags & MSI_FLAG_MULTI_PCI_MSI))
return 1;
@@ -1394,7 +1386,7 @@ int pci_msi_domain_check_cap(struct irq_domain *domain,
static int pci_msi_domain_handle_error(struct irq_domain *domain,
struct msi_desc *desc, int error)
{
- /* Special handling to support pci_enable_msi_range() */
+ /* Special handling to support __pci_enable_msi_range() */
if (pci_msi_desc_is_multi_msi(desc) && error == -ENOSPC)
return 1;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index e2d1a12..2159376 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1309,14 +1309,7 @@ void pci_msix_shutdown(struct pci_dev *dev);
void pci_disable_msix(struct pci_dev *dev);
void pci_restore_msi_state(struct pci_dev *dev);
int pci_msi_enabled(void);
-int pci_enable_msi_range(struct pci_dev *dev, int minvec, int maxvec);
-static inline int pci_enable_msi_exact(struct pci_dev *dev, int nvec)
-{
- int rc = pci_enable_msi_range(dev, nvec, nvec);
- if (rc < 0)
- return rc;
- return 0;
-}
+int pci_enable_msi(struct pci_dev *dev);
int pci_enable_msix_range(struct pci_dev *dev, struct msix_entry *entries,
int minvec, int maxvec);
static inline int pci_enable_msix_exact(struct pci_dev *dev,
@@ -1347,10 +1340,7 @@ static inline void pci_msix_shutdown(struct pci_dev *dev) { }
static inline void pci_disable_msix(struct pci_dev *dev) { }
static inline void pci_restore_msi_state(struct pci_dev *dev) { }
static inline int pci_msi_enabled(void) { return 0; }
-static inline int pci_enable_msi_range(struct pci_dev *dev, int minvec,
- int maxvec)
-{ return -ENOSYS; }
-static inline int pci_enable_msi_exact(struct pci_dev *dev, int nvec)
+static inline int pci_enable_msi(struct pci_dev *dev)
{ return -ENOSYS; }
static inline int pci_enable_msix_range(struct pci_dev *dev,
struct msix_entry *entries, int minvec, int maxvec)
@@ -1426,8 +1416,6 @@ static inline void pcie_set_ecrc_checking(struct pci_dev *dev) { }
static inline void pcie_ecrc_get_policy(char *str) { }
#endif
-#define pci_enable_msi(pdev) pci_enable_msi_exact(pdev, 1)
-
#ifdef CONFIG_HT_IRQ
/* The functions a driver should call */
int ht_create_irq(struct pci_dev *dev, int idx);
--
2.1.4
^ permalink raw reply related
* [PATCH 1/3] media/cobalt: use pci_irq_allocate_vectors
From: Christoph Hellwig @ 2017-01-09 20:37 UTC (permalink / raw)
To: linux-pci; +Cc: Mauro Carvalho Chehab, netdev, linux-media
In-Reply-To: <1483994260-19797-1-git-send-email-hch@lst.de>
Simply the interrupt setup by using the new PCI layer helpers.
Despite using pci_enable_msi_range, this driver was only requesting a
single MSI vector anyway.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/media/pci/cobalt/cobalt-driver.c | 8 ++------
drivers/media/pci/cobalt/cobalt-driver.h | 2 --
2 files changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/media/pci/cobalt/cobalt-driver.c b/drivers/media/pci/cobalt/cobalt-driver.c
index 9796340..d5c911c 100644
--- a/drivers/media/pci/cobalt/cobalt-driver.c
+++ b/drivers/media/pci/cobalt/cobalt-driver.c
@@ -308,9 +308,7 @@ static void cobalt_pci_iounmap(struct cobalt *cobalt, struct pci_dev *pci_dev)
static void cobalt_free_msi(struct cobalt *cobalt, struct pci_dev *pci_dev)
{
free_irq(pci_dev->irq, (void *)cobalt);
-
- if (cobalt->msi_enabled)
- pci_disable_msi(pci_dev);
+ pci_free_irq_vectors(pci_dev);
}
static int cobalt_setup_pci(struct cobalt *cobalt, struct pci_dev *pci_dev,
@@ -387,14 +385,12 @@ static int cobalt_setup_pci(struct cobalt *cobalt, struct pci_dev *pci_dev,
from being generated. */
cobalt_set_interrupt(cobalt, false);
- if (pci_enable_msi_range(pci_dev, 1, 1) < 1) {
+ if (pci_alloc_irq_vectors(pci_dev, 1, 1, PCI_IRQ_MSI) < 1) {
cobalt_err("Could not enable MSI\n");
- cobalt->msi_enabled = false;
ret = -EIO;
goto err_release;
}
msi_config_show(cobalt, pci_dev);
- cobalt->msi_enabled = true;
/* Register IRQ */
if (request_irq(pci_dev->irq, cobalt_irq_handler, IRQF_SHARED,
diff --git a/drivers/media/pci/cobalt/cobalt-driver.h b/drivers/media/pci/cobalt/cobalt-driver.h
index ed00dc9..00f773e 100644
--- a/drivers/media/pci/cobalt/cobalt-driver.h
+++ b/drivers/media/pci/cobalt/cobalt-driver.h
@@ -287,8 +287,6 @@ struct cobalt {
u32 irq_none;
u32 irq_full_fifo;
- bool msi_enabled;
-
/* omnitek dma */
int dma_channels;
int first_fifo_channel;
--
2.1.4
^ permalink raw reply related
* Re: [PATCH v2] phy state machine: failsafe leave invalid RUNNING state
From: David Miller @ 2017-01-09 20:38 UTC (permalink / raw)
To: zefir.kurtisi; +Cc: netdev, f.fainelli, andrew
In-Reply-To: <1483701288-14019-1-git-send-email-zefir.kurtisi@neratec.com>
From: Zefir Kurtisi <zefir.kurtisi@neratec.com>
Date: Fri, 6 Jan 2017 12:14:48 +0100
> While in RUNNING state, phy_state_machine() checks for link changes by
> comparing phydev->link before and after calling phy_read_status().
> This works as long as it is guaranteed that phydev->link is never
> changed outside the phy_state_machine().
>
> If in some setups this happens, it causes the state machine to miss
> a link loss and remain RUNNING despite phydev->link being 0.
>
> This has been observed running a dsa setup with a process continuously
> polling the link states over ethtool each second (SNMPD RFC-1213
> agent). Disconnecting the link on a phy followed by a ETHTOOL_GSET
> causes dsa_slave_get_settings() / dsa_slave_get_link_ksettings() to
> call phy_read_status() and with that modify the link status - and
> with that bricking the phy state machine.
>
> This patch adds a fail-safe check while in RUNNING, which causes to
> move to CHANGELINK when the link is gone and we are still RUNNING.
>
> Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
Patch applied, thanks.
^ permalink raw reply
* Re: pull-request: mac80211 2017-01-06
From: David Miller @ 2017-01-09 20:39 UTC (permalink / raw)
To: johannes; +Cc: netdev, linux-wireless
In-Reply-To: <20170106123721.10970-1-johannes@sipsolutions.net>
From: Johannes Berg <johannes@sipsolutions.net>
Date: Fri, 6 Jan 2017 13:37:20 +0100
> Since the new patch would otherwise cause conflicts, it might be good
> to pull net or Linus's next RC containing it into net-next, if you can.
Well, needed or not, it is done now :-)
^ permalink raw reply
* Re: [PATCH 0/2] sh_eth: "intgelligent checksum" related cleanups
From: David Miller @ 2017-01-09 20:41 UTC (permalink / raw)
To: sergei.shtylyov; +Cc: netdev, linux-renesas-soc
In-Reply-To: <2397522.NNiHGddh8y@wasted.cogentembedded.com>
From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date: Sat, 07 Jan 2017 00:01:40 +0300
> Here's a set of 2 patches against DaveM's 'net.git' repo, as they are based
> on a couple patches merged there recently; however, the patches are destined
> for 'net-next.git' (once 'net.git' gets merged there next time). I'm cleaning
> up the "intelligent checksum" related code (however, the driver only disables
> this feature for now, theres's no proper offload supprt yet).
Series applied to net-next, thanks.
^ permalink raw reply
* Re: [GIT] Networking
From: Kalle Valo @ 2017-01-09 20:44 UTC (permalink / raw)
To: David Miller; +Cc: torvalds, Larry.Finger, akpm, netdev, linux-kernel
In-Reply-To: <20170109.152437.40458564953109123.davem@davemloft.net>
David Miller <davem@davemloft.net> writes:
> From: Linus Torvalds <torvalds@linux-foundation.org>
> Date: Mon, 9 Jan 2017 12:08:02 -0800
>
>> On Sun, Jan 8, 2017 at 7:38 PM, David Miller <davem@davemloft.net> wrote:
>>>
>>> git://git.kernel.org/pub/scm/linux/kernel/git/davem/net
>>
>> Hmm. This still doesn't contain the rtlwifi oops fix that was posted
>> back before christmas.
>>
>> Kalle said it was applied to the wireless-drivers tree as commit
>> 60f59ce02785 in his tree, but it's never gotten to me.
>>
>> What's up? It's three weeks later, and people are hitting the bug.
>
> I haven't received a bug fix pull from Kalle since Dec 22nd:
>
> http://patchwork.ozlabs.org/patch/708247/
>
> That one has:
>
> rtlwifi: Fix kernel oops introduced with commit e49656147359
>
> Is that what you're talking about?
>
> In the thread I said I pulled it but indeed I don't have that commit
> in my tree either, weird.
>
> I just tried to pull that git URL right now and I get nothing:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers.git tags/wireless-drivers-for-davem-2016-12-22
>
> It looks like the rtlwifi fix is in his "pending" branch.
Be careful, there are two recent rtlwifi fixes in wireless-drivers:
60f59ce02785 rtlwifi: rtl_usb: Fix missing entry in USB driver's private data
22b68b93ae25 rtlwifi: Fix kernel oops introduced with commit e49656147359
You should have 22b68b93ae25 in your tree, but not 60f59ce02785. The
reason why you didn't get 60f59ce02785 in my pull request was that I
committed it late:
commit 60f59ce0278557f7896d5158ae6d12a4855a72cc
Author: Larry Finger <Larry.Finger@lwfinger.net>
AuthorDate: Wed Dec 21 11:18:55 2016 -0600
Commit: Kalle Valo <kvalo@codeaurora.org>
CommitDate: Fri Dec 30 15:38:13 2016 +0200
I'll send you a new pull request tomorrow (my time).
--
Kalle Valo
^ permalink raw reply
* Re: [RFC PATCH 2/4] page_pool: basic implementation of page_pool
From: Jesper Dangaard Brouer @ 2017-01-09 20:45 UTC (permalink / raw)
To: Vlastimil Babka
Cc: linux-mm, Alexander Duyck, willemdebruijn.kernel, netdev,
john.fastabend, Saeed Mahameed, bjorn.topel, Alexei Starovoitov,
Tariq Toukan, Mel Gorman, brouer
In-Reply-To: <38d42210-de93-f16f-fa54-b149127fffeb@suse.cz>
On Mon, 9 Jan 2017 11:43:39 +0100 Vlastimil Babka <vbabka@suse.cz> wrote:
> On 01/04/2017 12:00 PM, Jesper Dangaard Brouer wrote:
> >
> > On Tue, 3 Jan 2017 17:07:49 +0100 Vlastimil Babka <vbabka@suse.cz> wrote:
> >
> >> On 12/20/2016 02:28 PM, Jesper Dangaard Brouer wrote:
> >>> The focus in this patch is getting the API around page_pool figured out.
> >>>
> >>> The internal data structures for returning page_pool pages is not optimal.
> >>> This implementation use ptr_ring for recycling, which is known not to scale
> >>> in case of multiple remote CPUs releasing/returning pages.
> >>
> >> Just few very quick impressions...
> >>
> >>> A bulking interface into the page allocator is also left for later. (This
> >>> requires cooperation will Mel Gorman, who just send me some PoC patches for this).
> >>> ---
> > [...]
> >>> diff --git a/include/linux/mm.h b/include/linux/mm.h
> >>> index 4424784ac374..11b4d8fb280b 100644
> >>> --- a/include/linux/mm.h
> >>> +++ b/include/linux/mm.h
> > [...]
> >>> @@ -765,6 +766,11 @@ static inline void put_page(struct page *page)
> >>> {
> >>> page = compound_head(page);
> >>>
> >>> + if (PagePool(page)) {
> >>> + page_pool_put_page(page);
> >>> + return;
> >>> + }
> >>
> >> Can't say I'm thrilled about a new page flag and a test in put_page().
> >
> > In patch 4/4, I'm scaling this back. Avoiding to modify the inlined
> > put_page(), by letting refcnt reach zero and catching pages belonging to
> > a page_pool in __free_pages_ok() and free_hot_cold_page(). (Result
> > in being more dependent on page-refcnt and loosing some performance).
> >
> > Still needing a new page flag, or some other method of identifying when
> > a page belongs to a page_pool.
>
> I see. I guess if all page pool pages were order>0 compound pages, you
> could hook this to the existing compound_dtor functionality instead.
The page_pool will support order>0 pages, but it is the order-0 case
that is optimized for.
> >> I don't know the full life cycle here, but isn't it that these pages
> >> will be specifically allocated and used in page pool aware drivers,
> >> so maybe they can be also specifically freed there without hooking to
> >> the generic page refcount mechanism?
> >
> > Drivers are already manipulating refcnt, to "splitup" the page (to
> > save memory) for storing more RX frames per page. Which is something
> > the page_pool still need to support. (XDP can request one page per
> > packet and gain the direct recycle optimization and instead waste mem).
> >
> > Notice, a page_pool aware driver doesn't handle the "free-side". Free
> > happens when the packet/page is being consumed, spliced or transmitted
> > out another non-page_pool-aware NIC driver. An interresting case is
> > packet-page waiting for DMA TX completion (on another NIC), thus need
> > to async-store info on page_pool and DMA-addr.
> >
> > Could extend the SKB (with a page_pool pointer)... BUT it defeats the
> > purpose of avoiding to allocate the SKB. E.g. in the cases where XDP
> > takes the route-decision and transmit/forward the "raw"-page (out
> > another NIC or into a "raw" socket), then we don't have a meta-data
> > structure to store this info in. Thus, this info is stored in struct
> > page.
>
> OK.
>
> >>> + */
> >>> struct address_space *mapping; /* If low bit clear, points to
> >>> * inode address_space, or NULL.
> >>> * If page mapped as anonymous
> >>> @@ -63,6 +69,7 @@ struct page {
> >>> union {
> >>> pgoff_t index; /* Our offset within mapping. */
> >>> void *freelist; /* sl[aou]b first free object */
> >>> + dma_addr_t dma_addr; /* used by page_pool */
> >>> /* page_deferred_list().prev -- second tail page */
> >>> };
> >>>
> >>> @@ -117,6 +124,8 @@ struct page {
> >>> * avoid collision and false-positive PageTail().
> >>> */
> >>> union {
> >>> + /* XXX: Idea reuse lru list, in page_pool to align with PCP */
> >>> +
> >>> struct list_head lru; /* Pageout list, eg. active_list
> >>> * protected by zone_lru_lock !
> >>> * Can be used as a generic list
> >
> > Guess, I can move it here, as the page cannot be on the LRU-list, while
> > being used (or VMA mapped). Right?
>
> Well typically the VMA mapped pages are those on the LRU list (anonymous
> or file). But I don't suppose you will want memory reclaim to free your
> pages, so seems lru field should be reusable for you.
Thanks for the info.
So, LRU-list area could be reusable, but I does not align so well with
the bulking API Mel just introduced/proposed, but still doable.
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* [PATCH net-next v2 0/4] net: switchdev: Avoid sleep in atomic with DSA
From: Florian Fainelli @ 2017-01-09 20:45 UTC (permalink / raw)
To: netdev
Cc: davem, vivien.didelot, andrew, jiri, marcelo.leitner,
Florian Fainelli
Hi all,
This patch series is to resolve a sleeping function called in atomic context
debug splat that we observe with DSA.
Let me know what you think, I was also wondering if we should just always
make switchdev_port_vlan_fill() set SWITCHDEV_F_DEFER, but was afraid this
could cause invalid contexts to be used for rocker, mlxsw, i40e etc.
Changes in v2:
- Make patch 3 actually build, thanks to Marcelo for reporting the
problem
Thanks!
Florian Fainelli (4):
net: switchdev: Prepare for deferred functions modifying objects
net: switchdev: Add object dump deferred operation
net: switchdev: Add switchdev_port_bridge_getlink_deferred
net: dsa: Utilize switchdev_port_bridge_getlink_deferred()
include/net/switchdev.h | 3 +
net/dsa/slave.c | 2 +-
net/switchdev/switchdev.c | 171 +++++++++++++++++++++++++++++++++++++---------
3 files changed, 141 insertions(+), 35 deletions(-)
--
2.9.3
^ permalink raw reply
* Re: [PATCH net-next 0/4] net: dsa: Make dsa_switch_ops const
From: David Miller @ 2017-01-09 20:45 UTC (permalink / raw)
To: f.fainelli; +Cc: netdev, andrew, vivien.didelot
In-Reply-To: <20170108225208.15431-1-f.fainelli@gmail.com>
From: Florian Fainelli <f.fainelli@gmail.com>
Date: Sun, 8 Jan 2017 14:52:04 -0800
> This patch series allows us to annotate dsa_switch_ops with a const
> qualifier.
Series applied, thanks.
^ permalink raw reply
* [PATCH net-next v2 1/4] net: switchdev: Prepare for deferred functions modifying objects
From: Florian Fainelli @ 2017-01-09 20:45 UTC (permalink / raw)
To: netdev
Cc: davem, vivien.didelot, andrew, jiri, marcelo.leitner,
Florian Fainelli
In-Reply-To: <20170109204523.5843-1-f.fainelli@gmail.com>
In preparation for adding support for deferred dump operations, allow
specifying a deferred function whose signature allows read/write
objects.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
net/switchdev/switchdev.c | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
index 017801f9dbaa..3d70ad02c617 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -100,11 +100,14 @@ static DEFINE_SPINLOCK(deferred_lock);
typedef void switchdev_deferred_func_t(struct net_device *dev,
const void *data);
+typedef void switchdev_deferred_func_rw_t(struct net_device *dev,
+ void *data);
struct switchdev_deferred_item {
struct list_head list;
struct net_device *dev;
switchdev_deferred_func_t *func;
+ switchdev_deferred_func_rw_t *func_rw;
unsigned long data[0];
};
@@ -138,7 +141,10 @@ void switchdev_deferred_process(void)
ASSERT_RTNL();
while ((dfitem = switchdev_deferred_dequeue())) {
- dfitem->func(dfitem->dev, dfitem->data);
+ if (dfitem->func)
+ dfitem->func(dfitem->dev, dfitem->data);
+ else if (dfitem->func_rw)
+ dfitem->func_rw(dfitem->dev, dfitem->data);
dev_put(dfitem->dev);
kfree(dfitem);
}
@@ -156,7 +162,8 @@ static DECLARE_WORK(deferred_process_work, switchdev_deferred_process_work);
static int switchdev_deferred_enqueue(struct net_device *dev,
const void *data, size_t data_len,
- switchdev_deferred_func_t *func)
+ switchdev_deferred_func_t *func,
+ switchdev_deferred_func_rw_t *func_rw)
{
struct switchdev_deferred_item *dfitem;
@@ -165,6 +172,7 @@ static int switchdev_deferred_enqueue(struct net_device *dev,
return -ENOMEM;
dfitem->dev = dev;
dfitem->func = func;
+ dfitem->func_rw = func_rw;
memcpy(dfitem->data, data, data_len);
dev_hold(dev);
spin_lock_bh(&deferred_lock);
@@ -312,7 +320,8 @@ static int switchdev_port_attr_set_defer(struct net_device *dev,
const struct switchdev_attr *attr)
{
return switchdev_deferred_enqueue(dev, attr, sizeof(*attr),
- switchdev_port_attr_set_deferred);
+ switchdev_port_attr_set_deferred,
+ NULL);
}
/**
@@ -441,7 +450,8 @@ static int switchdev_port_obj_add_defer(struct net_device *dev,
const struct switchdev_obj *obj)
{
return switchdev_deferred_enqueue(dev, obj, switchdev_obj_size(obj),
- switchdev_port_obj_add_deferred);
+ switchdev_port_obj_add_deferred,
+ NULL);
}
/**
@@ -511,7 +521,8 @@ static int switchdev_port_obj_del_defer(struct net_device *dev,
const struct switchdev_obj *obj)
{
return switchdev_deferred_enqueue(dev, obj, switchdev_obj_size(obj),
- switchdev_port_obj_del_deferred);
+ switchdev_port_obj_del_deferred,
+ NULL);
}
/**
--
2.9.3
^ permalink raw reply related
* [PATCH net-next v2 2/4] net: switchdev: Add object dump deferred operation
From: Florian Fainelli @ 2017-01-09 20:45 UTC (permalink / raw)
To: netdev
Cc: davem, vivien.didelot, andrew, jiri, marcelo.leitner,
Florian Fainelli
In-Reply-To: <20170109204523.5843-1-f.fainelli@gmail.com>
Add plumbing required to perform dump operations in a deferred context,
this mostly wraps the existing switchdev_obj_port_dump() function into a
switchdev_obj_port_dump_now() and adds two wrappers (normal and
deferred) around.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
net/switchdev/switchdev.c | 71 +++++++++++++++++++++++++++++++++++++----------
1 file changed, 57 insertions(+), 14 deletions(-)
diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
index 3d70ad02c617..4fa9972d72d2 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -545,26 +545,15 @@ int switchdev_port_obj_del(struct net_device *dev,
}
EXPORT_SYMBOL_GPL(switchdev_port_obj_del);
-/**
- * switchdev_port_obj_dump - Dump port objects
- *
- * @dev: port device
- * @id: object ID
- * @obj: object to dump
- * @cb: function to call with a filled object
- *
- * rtnl_lock must be held.
- */
-int switchdev_port_obj_dump(struct net_device *dev, struct switchdev_obj *obj,
- switchdev_obj_dump_cb_t *cb)
+static int switchdev_port_obj_dump_now(struct net_device *dev,
+ struct switchdev_obj *obj,
+ switchdev_obj_dump_cb_t *cb)
{
const struct switchdev_ops *ops = dev->switchdev_ops;
struct net_device *lower_dev;
struct list_head *iter;
int err = -EOPNOTSUPP;
- ASSERT_RTNL();
-
if (ops && ops->switchdev_port_obj_dump)
return ops->switchdev_port_obj_dump(dev, obj, cb);
@@ -580,6 +569,60 @@ int switchdev_port_obj_dump(struct net_device *dev, struct switchdev_obj *obj,
return err;
}
+
+struct switchdev_obj_dump_deferred_item {
+ struct switchdev_obj obj;
+ switchdev_obj_dump_cb_t *cb;
+};
+
+static void switchdev_port_obj_dump_deferred(struct net_device *dev,
+ void *data)
+{
+ struct switchdev_obj_dump_deferred_item *i = data;
+ struct switchdev_obj *obj = &i->obj;
+ int err;
+
+ err = switchdev_port_obj_dump_now(dev, obj, i->cb);
+ if (err && err != -EOPNOTSUPP)
+ netdev_err(dev, "failed (err=%d) to dump object (id=%d)\n",
+ err, obj->id);
+ if (obj->complete)
+ obj->complete(dev, err, obj->complete_priv);
+}
+
+static int switchdev_port_obj_dump_defer(struct net_device *dev,
+ struct switchdev_obj *obj,
+ switchdev_obj_dump_cb_t *cb)
+{
+ struct switchdev_obj_dump_deferred_item item;
+
+ memcpy(&item.obj, obj, switchdev_obj_size(obj));
+ item.cb = cb;
+
+ return switchdev_deferred_enqueue(dev, &item, sizeof(item),
+ NULL,
+ switchdev_port_obj_dump_deferred);
+}
+
+/**
+ * switchdev_port_obj_dump - Dump port objects
+ *
+ * @dev: port device
+ * @id: object ID
+ * @obj: object to dump
+ * @cb: function to call with a filled object
+ *
+ * rtnl_lock must be held and must not be in atomic section,
+ * in case SWITCHDEV_F_DEFER flag is not set.
+ */
+int switchdev_port_obj_dump(struct net_device *dev, struct switchdev_obj *obj,
+ switchdev_obj_dump_cb_t *cb)
+{
+ if (obj->flags & SWITCHDEV_F_DEFER)
+ return switchdev_port_obj_dump_defer(dev, obj, cb);
+ ASSERT_RTNL();
+ return switchdev_port_obj_dump_now(dev, obj, cb);
+}
EXPORT_SYMBOL_GPL(switchdev_port_obj_dump);
static RAW_NOTIFIER_HEAD(switchdev_notif_chain);
--
2.9.3
^ permalink raw reply related
* [PATCH net-next v2 3/4] net: switchdev: Add switchdev_port_bridge_getlink_deferred
From: Florian Fainelli @ 2017-01-09 20:45 UTC (permalink / raw)
To: netdev
Cc: davem, vivien.didelot, andrew, jiri, marcelo.leitner,
Florian Fainelli
In-Reply-To: <20170109204523.5843-1-f.fainelli@gmail.com>
Add switchdev_port_bridge_getlink_deferred() which does a deferred
object dump operation, this is required for e.g: DSA switches which
typically have sleeping I/O operations which is incompatible with being
in atomic context obviously.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
include/net/switchdev.h | 3 ++
net/switchdev/switchdev.c | 77 ++++++++++++++++++++++++++++++++++++++---------
2 files changed, 66 insertions(+), 14 deletions(-)
diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index eba80c4fc56f..087761b0df49 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -189,6 +189,9 @@ int call_switchdev_notifiers(unsigned long val, struct net_device *dev,
int switchdev_port_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
struct net_device *dev, u32 filter_mask,
int nlflags);
+int switchdev_port_bridge_getlink_deferred(struct sk_buff *skb, u32 pid,
+ u32 seq, struct net_device *dev,
+ u32 filter_mask, int nlflags);
int switchdev_port_bridge_setlink(struct net_device *dev,
struct nlmsghdr *nlh, u16 flags);
int switchdev_port_bridge_dellink(struct net_device *dev,
diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
index 4fa9972d72d2..f5db799fc0a0 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -776,12 +776,14 @@ static int switchdev_port_vlan_dump_cb(struct switchdev_obj *obj)
return err;
}
-static int switchdev_port_vlan_fill(struct sk_buff *skb, struct net_device *dev,
- u32 filter_mask)
+static int __switchdev_port_vlan_fill(struct sk_buff *skb,
+ struct net_device *dev,
+ u32 filter_mask, u32 obj_flags)
{
struct switchdev_vlan_dump dump = {
.vlan.obj.orig_dev = dev,
.vlan.obj.id = SWITCHDEV_OBJ_ID_PORT_VLAN,
+ .vlan.obj.flags = obj_flags,
.skb = skb,
.filter_mask = filter_mask,
};
@@ -802,17 +804,27 @@ static int switchdev_port_vlan_fill(struct sk_buff *skb, struct net_device *dev,
return err == -EOPNOTSUPP ? 0 : err;
}
-/**
- * switchdev_port_bridge_getlink - Get bridge port attributes
- *
- * @dev: port device
- *
- * Called for SELF on rtnl_bridge_getlink to get bridge port
- * attributes.
- */
-int switchdev_port_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
- struct net_device *dev, u32 filter_mask,
- int nlflags)
+static int switchdev_port_vlan_fill_deferred(struct sk_buff *skb,
+ struct net_device *dev,
+ u32 filter_mask)
+{
+ return __switchdev_port_vlan_fill(skb, dev, filter_mask,
+ SWITCHDEV_F_DEFER);
+}
+
+static int switchdev_port_vlan_fill(struct sk_buff *skb,
+ struct net_device *dev,
+ u32 filter_mask)
+{
+ return __switchdev_port_vlan_fill(skb, dev, filter_mask, 0);
+}
+
+static int __switchdev_port_bridge_getlink(struct sk_buff *skb, u32 pid,
+ u32 seq, struct net_device *dev,
+ u32 filter_mask, int nlflags,
+ int (*fill_cb)(struct sk_buff *skb,
+ struct net_device *d,
+ u32 filter_mask))
{
struct switchdev_attr attr = {
.orig_dev = dev,
@@ -831,10 +843,47 @@ int switchdev_port_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
return ndo_dflt_bridge_getlink(skb, pid, seq, dev, mode,
attr.u.brport_flags, mask, nlflags,
- filter_mask, switchdev_port_vlan_fill);
+ filter_mask, fill_cb);
+}
+
+/**
+ * switchdev_port_bridge_getlink - Get bridge port attributes
+ *
+ * @dev: port device
+ *
+ * Called for SELF on rtnl_bridge_getlink to get bridge port
+ * attributes.
+ */
+int switchdev_port_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
+ struct net_device *dev, u32 filter_mask,
+ int nlflags)
+{
+ return __switchdev_port_bridge_getlink(skb, pid, seq, dev, filter_mask,
+ nlflags,
+ switchdev_port_vlan_fill);
}
EXPORT_SYMBOL_GPL(switchdev_port_bridge_getlink);
+/**
+ * switchdev_port_bridge_getlink_deferred - Get bridge port attributes
+ * (deferred variant)
+ *
+ * @dev: port device
+ *
+ * Called for SELF on rtnl_bridge_getlink to get bridge port
+ * attributes.
+ */
+int switchdev_port_bridge_getlink_deferred(struct sk_buff *skb, u32 pid,
+ u32 seq, struct net_device *dev,
+ u32 filter_mask,
+ int nlflags)
+{
+ return __switchdev_port_bridge_getlink(skb, pid, seq, dev, filter_mask,
+ nlflags,
+ switchdev_port_vlan_fill_deferred);
+}
+EXPORT_SYMBOL_GPL(switchdev_port_bridge_getlink_deferred);
+
static int switchdev_port_br_setflag(struct net_device *dev,
struct nlattr *nlattr,
unsigned long brport_flag)
--
2.9.3
^ permalink raw reply related
* [PATCH net-next v2 4/4] net: dsa: Utilize switchdev_port_bridge_getlink_deferred()
From: Florian Fainelli @ 2017-01-09 20:45 UTC (permalink / raw)
To: netdev
Cc: davem, vivien.didelot, andrew, jiri, marcelo.leitner,
Florian Fainelli
In-Reply-To: <20170109204523.5843-1-f.fainelli@gmail.com>
Fixes the following sleeping in atomic splat:
[ 69.008021] BUG: sleeping function called from invalid context at kernel/locking/mutex.c:752
[ 69.016523] in_atomic(): 1, irqs_disabled(): 0, pid: 1528, name: bridge
[ 69.023167] INFO: lockdep is turned off.
[ 69.027118] CPU: 1 PID: 1528 Comm: bridge Not tainted 4.10.0-rc2-00131-g719651624789-dirty #205
[ 69.035840] Hardware name: Broadcom STB (Flattened Device Tree)
[ 69.041796] [<c020fc40>] (unwind_backtrace) from [<c020ba28>] (show_stack+0x10/0x14)
[ 69.049570] [<c020ba28>] (show_stack) from [<c04fb91c>] (dump_stack+0xb0/0xdc)
[ 69.056823] [<c04fb91c>] (dump_stack) from [<c0244d00>] (___might_sleep+0x1a4/0x2a4)
[ 69.064599] [<c0244d00>] (___might_sleep) from [<c08f3eb8>] (mutex_lock_nested+0x28/0x7d8)
[ 69.072897] [<c08f3eb8>] (mutex_lock_nested) from [<c0674dbc>] (b53_vlan_dump+0x2c/0x104)
[ 69.081105] [<c0674dbc>] (b53_vlan_dump) from [<c08eba88>] (switchdev_port_obj_dump_now+0x30/0x6c)
[ 69.090094] [<c08eba88>] (switchdev_port_obj_dump_now) from [<c08ebb00>] (switchdev_port_obj_dump+0x3c/0x98)
[ 69.099950] [<c08ebb00>] (switchdev_port_obj_dump) from [<c08ebc34>] (switchdev_port_vlan_fill+0x68/0x90)
[ 69.109550] [<c08ebc34>] (switchdev_port_vlan_fill) from [<c07f13a4>] (ndo_dflt_bridge_getlink+0x28c/0x4fc)
[ 69.119320] [<c07f13a4>] (ndo_dflt_bridge_getlink) from [<c08eb2c8>] (switchdev_port_bridge_getlink+0xc4/0xd4)
[ 69.129350] [<c08eb2c8>] (switchdev_port_bridge_getlink) from [<c07f0b10>] (rtnl_bridge_getlink+0x12c/0x28c)
[ 69.139206] [<c07f0b10>] (rtnl_bridge_getlink) from [<c0802e28>] (netlink_dump+0xe8/0x268)
[ 69.147495] [<c0802e28>] (netlink_dump) from [<c0803864>] (__netlink_dump_start+0x12c/0x18c)
[ 69.155958] [<c0803864>] (__netlink_dump_start) from [<c07f3c34>] (rtnetlink_rcv_msg+0x11c/0x228)
[ 69.164857] [<c07f3c34>] (rtnetlink_rcv_msg) from [<c0805e3c>] (netlink_rcv_skb+0xc4/0xd8)
[ 69.173145] [<c0805e3c>] (netlink_rcv_skb) from [<c07f1110>] (rtnetlink_rcv+0x28/0x30)
[ 69.181087] [<c07f1110>] (rtnetlink_rcv) from [<c080577c>] (netlink_unicast+0x16c/0x238)
[ 69.189201] [<c080577c>] (netlink_unicast) from [<c0805c3c>] (netlink_sendmsg+0x350/0x364)
[ 69.197493] [<c0805c3c>] (netlink_sendmsg) from [<c07beab0>] (sock_sendmsg+0x14/0x24)
[ 69.205350] [<c07beab0>] (sock_sendmsg) from [<c07bfdbc>] (SyS_sendto+0xb8/0xe0)
[ 69.212769] [<c07bfdbc>] (SyS_sendto) from [<c07bfdfc>] (SyS_send+0x18/0x20)
[ 69.219841] [<c07bfdfc>] (SyS_send) from [<c0208100>] (ret_fast_syscall+0x0/0x1c)
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
net/dsa/slave.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 5cd5b8137c08..b38536f951ea 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1038,7 +1038,7 @@ static const struct net_device_ops dsa_slave_netdev_ops = {
.ndo_netpoll_cleanup = dsa_slave_netpoll_cleanup,
.ndo_poll_controller = dsa_slave_poll_controller,
#endif
- .ndo_bridge_getlink = switchdev_port_bridge_getlink,
+ .ndo_bridge_getlink = switchdev_port_bridge_getlink_deferred,
.ndo_bridge_setlink = switchdev_port_bridge_setlink,
.ndo_bridge_dellink = switchdev_port_bridge_dellink,
.ndo_get_phys_port_id = dsa_slave_get_phys_port_id,
--
2.9.3
^ permalink raw reply related
* Re: [PATCH net-next 0/4] afs: Refcount afs_call struct
From: David Miller @ 2017-01-09 20:48 UTC (permalink / raw)
To: dhowells; +Cc: netdev, linux-afs, linux-kernel
In-Reply-To: <148397356909.20445.15077871371099721338.stgit@warthog.procyon.org.uk>
From: David Howells <dhowells@redhat.com>
Date: Mon, 09 Jan 2017 14:52:49 +0000
>
> These patches provide some tracepoints for AFS and fix a potential leak by
> adding refcounting to the afs_call struct.
>
> The patches are:
>
> (1) Add some tracepoints for logging incoming calls and monitoring
> notifications from AF_RXRPC and data reception.
>
> (2) Get rid of afs_wait_mode as it didn't turn out to be as useful as
> initially expected. It can be brought back later if needed. This
> clears some stuff out that I don't then need to fix up in (4).
>
> (3) Allow listen(..., 0) to be used to disable listening. This makes
> shutting down the AFS cache manager server in the kernel much easier
> and the accounting simpler as we can then be sure that (a) all
> preallocated afs_call structs are relesed and (b) no new incoming
> calls are going to be started.
>
> For the moment, listening cannot be reenabled.
>
> (4) Add refcounting to the afs_call struct to fix a potential multiple
> release detected by static checking and add a tracepoint to follow the
> lifecycle of afs_call objects.
>
> The patches can be found here also:
>
> http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=rxrpc-rewrite
>
> Tagged thusly:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git
> rxrpc-rewrite-20170109
Pulled, thanks David.
^ permalink raw reply
* Re: [PATCH net-next 0/4] net: switchdev: Avoid sleep in atomic with DSA
From: Ido Schimmel @ 2017-01-09 20:48 UTC (permalink / raw)
To: Florian Fainelli; +Cc: netdev, davem, vivien.didelot, andrew, jiri
In-Reply-To: <20170109194503.10713-1-f.fainelli@gmail.com>
Hi Florian,
On Mon, Jan 09, 2017 at 11:44:59AM -0800, Florian Fainelli wrote:
> Hi all,
>
> This patch series is to resolve a sleeping function called in atomic context
> debug splat that we observe with DSA.
>
> Let me know what you think, I was also wondering if we should just always
> make switchdev_port_vlan_fill() set SWITCHDEV_F_DEFER, but was afraid this
> could cause invalid contexts to be used for rocker, mlxsw, i40e etc.
Isn't this a bit of overkill? All the drivers you mention fill the VLAN
dump from their cache and don't require sleeping. Even b53 that you
mention in the last patch does that, but reads the PVID from the device,
which entails taking a mutex.
Can't you just cache the PVID as well? I think this will solve your
problem. Didn't look too much into the b53 code, so maybe I'm missing
something. Seems that mv88e6xxx has a similar problem.
Thanks!
^ permalink raw reply
* Re: [net-next PATCH 0/3] net: optimize ICMP-reply code path
From: David Miller @ 2017-01-09 20:49 UTC (permalink / raw)
To: brouer; +Cc: netdev, eric.dumazet, xiyou.wangcong
In-Reply-To: <20170109150246.30215.63371.stgit@firesoul>
From: Jesper Dangaard Brouer <brouer@redhat.com>
Date: Mon, 09 Jan 2017 16:03:59 +0100
> This patchset is optimizing the ICMP-reply code path, for ICMP packets
> that gets rate limited. A remote party can easily trigger this code
> path by sending packets to port number with no listening service.
>
> Generally the patchset moves the sysctl_icmp_msgs_per_sec ratelimit
> checking to earlier in the code path and removes an allocation.
>
>
> Use-case: The specific case I experienced this being a bottleneck is,
> sending UDP packets to a port with no listener, which obviously result
> in kernel replying with ICMP Destination Unreachable (type:3), Port
> Unreachable (code:3), which cause the bottleneck.
>
> After Eric and Paolo optimized the UDP socket code, the kernels PPS
> processing capabilities is lower for no-listen ports, than normal UDP
> sockets. This is bad for capacity planning when restarting a service.
>
> UDP no-listen benchmark 8xCPUs using pktgen_sample04_many_flows.sh:
> Baseline: 6.6 Mpps
> Patch: 14.7 Mpps
> Driver mlx5 at 50Gbit/s.
Series applied, thanks Jesper!
^ permalink raw reply
* Re: [PATCH net] net: dsa: Ensure validity of dst->ds[0]
From: Vivien Didelot @ 2017-01-09 20:50 UTC (permalink / raw)
To: Florian Fainelli, netdev; +Cc: davem, andrew, Florian Fainelli
In-Reply-To: <20170109195834.11697-1-f.fainelli@gmail.com>
Hi Florian,
Florian Fainelli <f.fainelli@gmail.com> writes:
> It is perfectly possible to have non zero indexed switches being present
> in a DSA switch tree, in such a case, we will be deferencing a NULL
> pointer while dsa_cpu_port_ethtool_{setup,restore}. Be more defensive
> and ensure that dst->ds[0] is valid before doing anything with it.
>
> Fixes: 0c73c523cf73 ("net: dsa: Initialize CPU port ethtool ops per tree")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
The patch is correct since we are already using dst->ds[0] here.
But we should stop using that and use dst->cpu_switch instead, because
the switch with ID 0 won't necessary be the CPU switch. Now that the
Ethernet switch chips are true Linux devices, they are registered in
order depending on their bus/address. So in a setup like this:
,--MDIO--@4--------@2--
| | |
[CPU] <-> [swA] <-> [swB]
swB will have index 0 and swA will have index 1. Please correct me if
I'm wrong.
Thanks,
Vivien
^ permalink raw reply
* Re: [net-next PATCH 1/3] Revert "icmp: avoid allocating large struct on stack"
From: Jesper Dangaard Brouer @ 2017-01-09 20:53 UTC (permalink / raw)
To: David Miller; +Cc: eric.dumazet, xiyou.wangcong, netdev, brouer
In-Reply-To: <20170109.135259.988711786570465428.davem@davemloft.net>
On Mon, 09 Jan 2017 13:52:59 -0500 (EST)
David Miller <davem@davemloft.net> wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Mon, 09 Jan 2017 10:07:04 -0800
>
> > You really should come to netdev conferences so that you understand
> > goals and efforts, instead of living in your cave.
>
> I completely agree with Eric.
>
> Cong we have a very serious problem with you exactly because you make
> quite vicious emotional statements targetted at other developers
> merely when they say something you disagree with.
>
> This is completely unacceptable behavior, and you must stop doing
> this, now.
I agree, and it is even documented in:
Documentation/process/code-of-conflict.rst
Quote: "As a reviewer of code, please strive to keep things civil and
focused on the technical issues involved. [...]"
https://www.kernel.org/doc/html/latest/process/code-of-conflict.html
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [PATCHv3 0/6] sh_eth: add wake-on-lan support via magic packet
From: David Miller @ 2017-01-09 20:55 UTC (permalink / raw)
To: niklas.soderlund+renesas
Cc: sergei.shtylyov, horms+renesas, netdev, linux-renesas-soc, geert,
linux-pm
In-Reply-To: <20170109153409.13956-1-niklas.soderlund+renesas@ragnatech.se>
From: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Date: Mon, 9 Jan 2017 16:34:03 +0100
> This series adds support for Wake-on-Lan using Magic Packet for a few
> models of the sh_eth driver. Patch 1/6 fix a naming error, patch 2/6
> adds generic support to control and support WoL while patches 3/6 - 6/6
> enable different models.
>
> Based ontop of net-next master.
...
Series applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next 0/4] net: switchdev: Avoid sleep in atomic with DSA
From: Florian Fainelli @ 2017-01-09 20:56 UTC (permalink / raw)
To: Ido Schimmel; +Cc: netdev, davem, vivien.didelot, andrew, jiri
In-Reply-To: <20170109204849.GA28310@splinter>
On 01/09/2017 12:48 PM, Ido Schimmel wrote:
> Hi Florian,
>
> On Mon, Jan 09, 2017 at 11:44:59AM -0800, Florian Fainelli wrote:
>> Hi all,
>>
>> This patch series is to resolve a sleeping function called in atomic context
>> debug splat that we observe with DSA.
>>
>> Let me know what you think, I was also wondering if we should just always
>> make switchdev_port_vlan_fill() set SWITCHDEV_F_DEFER, but was afraid this
>> could cause invalid contexts to be used for rocker, mlxsw, i40e etc.
>
> Isn't this a bit of overkill? All the drivers you mention fill the VLAN
> dump from their cache and don't require sleeping. Even b53 that you
> mention in the last patch does that, but reads the PVID from the device,
> which entails taking a mutex.
Correct.
>
> Can't you just cache the PVID as well? I think this will solve your
> problem. Didn't look too much into the b53 code, so maybe I'm missing
> something. Seems that mv88e6xxx has a similar problem.
I suppose we could indeed cache the PVID for b53, but for mv88e6xxx it
seems like we need to perform a bunch of VTU operations, and those
access HW registers, Andrew, Vivien, how do you want to solve that, do
we want to introduce a general VLAN cache somewhere in switchdev/DSA/driver?
Thanks Ido!
--
Florian
^ permalink raw reply
* Re: [PATCH net] net: dsa: Ensure validity of dst->ds[0]
From: Andrew Lunn @ 2017-01-09 21:01 UTC (permalink / raw)
To: Vivien Didelot; +Cc: Florian Fainelli, netdev, davem
In-Reply-To: <8737gsc5zm.fsf@weeman.i-did-not-set--mail-host-address--so-tickle-me>
On Mon, Jan 09, 2017 at 03:50:53PM -0500, Vivien Didelot wrote:
> Hi Florian,
>
> Florian Fainelli <f.fainelli@gmail.com> writes:
>
> > It is perfectly possible to have non zero indexed switches being present
> > in a DSA switch tree, in such a case, we will be deferencing a NULL
> > pointer while dsa_cpu_port_ethtool_{setup,restore}. Be more defensive
> > and ensure that dst->ds[0] is valid before doing anything with it.
> >
> > Fixes: 0c73c523cf73 ("net: dsa: Initialize CPU port ethtool ops per tree")
> > Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>
> Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
>
> The patch is correct since we are already using dst->ds[0] here.
>
> But we should stop using that and use dst->cpu_switch instead, because
> the switch with ID 0 won't necessary be the CPU switch. Now that the
> Ethernet switch chips are true Linux devices, they are registered in
> order depending on their bus/address. So in a setup like this:
>
> ,--MDIO--@4--------@2--
> | | |
> [CPU] <-> [swA] <-> [swB]
>
> swB will have index 0 and swA will have index 1. Please correct me if
> I'm wrong.
Correct, which DS has the CPU port is arbitrary.
It also gets messier when Johns finishes reworking my PoC patchset for
multiple CPU ports. You ideally want the correct CPU port for this
host interface.
Andrew
^ permalink raw reply
* Re: [PATCH net-next 0/4] net: switchdev: Avoid sleep in atomic with DSA
From: Andrew Lunn @ 2017-01-09 21:05 UTC (permalink / raw)
To: Ido Schimmel; +Cc: Florian Fainelli, netdev, davem, vivien.didelot, jiri
In-Reply-To: <20170109204849.GA28310@splinter>
On Mon, Jan 09, 2017 at 10:48:49PM +0200, Ido Schimmel wrote:
> Hi Florian,
>
> On Mon, Jan 09, 2017 at 11:44:59AM -0800, Florian Fainelli wrote:
> > Hi all,
> >
> > This patch series is to resolve a sleeping function called in atomic context
> > debug splat that we observe with DSA.
> >
> > Let me know what you think, I was also wondering if we should just always
> > make switchdev_port_vlan_fill() set SWITCHDEV_F_DEFER, but was afraid this
> > could cause invalid contexts to be used for rocker, mlxsw, i40e etc.
>
> Isn't this a bit of overkill? All the drivers you mention fill the VLAN
> dump from their cache and don't require sleeping.
Hi Ido
DSA in general does not cache information. It always ask the hardware.
So for mv88e6xxx, this is going to trigger MDIO operations, which take
mutex's and do sleep.
Andrew
^ permalink raw reply
* Re: [PATCH net-next 0/4] net: switchdev: Avoid sleep in atomic with DSA
From: Ido Schimmel @ 2017-01-09 21:14 UTC (permalink / raw)
To: Florian Fainelli; +Cc: netdev, davem, vivien.didelot, andrew, jiri
In-Reply-To: <e28f29c8-04ed-1d71-7b13-27a5d58d111c@gmail.com>
On Mon, Jan 09, 2017 at 12:56:48PM -0800, Florian Fainelli wrote:
> On 01/09/2017 12:48 PM, Ido Schimmel wrote:
> > Hi Florian,
> >
> > On Mon, Jan 09, 2017 at 11:44:59AM -0800, Florian Fainelli wrote:
> >> Hi all,
> >>
> >> This patch series is to resolve a sleeping function called in atomic context
> >> debug splat that we observe with DSA.
> >>
> >> Let me know what you think, I was also wondering if we should just always
> >> make switchdev_port_vlan_fill() set SWITCHDEV_F_DEFER, but was afraid this
> >> could cause invalid contexts to be used for rocker, mlxsw, i40e etc.
> >
> > Isn't this a bit of overkill? All the drivers you mention fill the VLAN
> > dump from their cache and don't require sleeping. Even b53 that you
> > mention in the last patch does that, but reads the PVID from the device,
> > which entails taking a mutex.
>
> Correct.
>
> >
> > Can't you just cache the PVID as well? I think this will solve your
> > problem. Didn't look too much into the b53 code, so maybe I'm missing
> > something. Seems that mv88e6xxx has a similar problem.
>
> I suppose we could indeed cache the PVID for b53, but for mv88e6xxx it
> seems like we need to perform a bunch of VTU operations, and those
> access HW registers, Andrew, Vivien, how do you want to solve that, do
> we want to introduce a general VLAN cache somewhere in switchdev/DSA/driver?
Truth be told, I don't quite understand why switchdev infra even tries
to dump the VLANs from the device. Like, in which situations is this
going to be different from what the software bridge reports? Sure, you
can set the VLAN filters with SELF and skip the software bridge, but how
does that make sense in a model where you want to reflect the software
datapath?
^ permalink raw reply
* Re: [PATCH net-next 0/4] net: switchdev: Avoid sleep in atomic with DSA
From: Vivien Didelot @ 2017-01-09 21:19 UTC (permalink / raw)
To: Florian Fainelli, Ido Schimmel; +Cc: netdev, davem, andrew, jiri
In-Reply-To: <e28f29c8-04ed-1d71-7b13-27a5d58d111c@gmail.com>
Hi Florian, Ido,
Florian Fainelli <f.fainelli@gmail.com> writes:
>> Can't you just cache the PVID as well? I think this will solve your
>> problem. Didn't look too much into the b53 code, so maybe I'm missing
>> something. Seems that mv88e6xxx has a similar problem.
>
> I suppose we could indeed cache the PVID for b53, but for mv88e6xxx it
> seems like we need to perform a bunch of VTU operations, and those
> access HW registers, Andrew, Vivien, how do you want to solve that, do
> we want to introduce a general VLAN cache somewhere in switchdev/DSA/driver?
Yes mv88e6xxx does read the hardware registers to get the port default
VID and read the hardware VLAN table.
DSA drivers should be dumb and simply implement switch operations. If
caching is required, it must be implemented in the DSA layer. Since
switchdev is stateless, I hardly see a way to implement it there.
Thanks,
Vivien
^ permalink raw reply
* Re: [PATCH net-next 0/4] net: switchdev: Avoid sleep in atomic with DSA
From: Andrew Lunn @ 2017-01-09 21:23 UTC (permalink / raw)
To: Ido Schimmel; +Cc: Florian Fainelli, netdev, davem, vivien.didelot, jiri
In-Reply-To: <20170109211436.GB28310@splinter>
> Truth be told, I don't quite understand why switchdev infra even tries
> to dump the VLANs from the device. Like, in which situations is this
> going to be different from what the software bridge reports?
What happens when the hardware is out of resources and says sorry,
cannot do that. There has been a few discussions about what to do in
that situation. Fall back to software, i.e. the software bridge does
it, or totally fail the operation. If you are failing back to
software, the states can be different.
Andrew
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox