* [PATCH net-next 2/3] ipv6: mld: do not overwrite uri when receiving an mldv2 query
From: Daniel Borkmann @ 2014-09-23 7:03 UTC (permalink / raw)
To: davem; +Cc: hannes, netdev
In-Reply-To: <1411455828-5196-1-git-send-email-dborkman@redhat.com>
While reviewing the code, I found it confusing why we update the URI when
receiving an MLDv2 query. The RFC does not mention any of this, and I also
double-checked with other implementations.
It is true that we start the general query timer with the received max_delay,
as mentioned in the older RFC2710, section 5.:
[...] "start timer" for the address on the interface, using a delay
value chosen uniformly from the interval [0, Maximum Response Delay],
where Maximum Response Delay is specified in the Query. If this is
an unsolicited Report, the timer is set to a delay value chosen
uniformly from the interval [0, [Unsolicited Report Interval] ].
It however does not say anywhere that we are supposed to overwrite that
value. The purpose of the report is quite different and described as:
When a node starts listening to a multicast address on an interface,
it should immediately transmit an unsolicited Report for that address
on that interface, in case it is the first listener on the link.
To cover the possibility of the initial Report being lost or damaged,
it is recommended that it be repeated once or twice after short delays
[Unsolicited Report Interval]. (A simple way to accomplish this is to
send the initial Report and then act as if a Multicast-Address-Specific
Query was received for that address, and set a timer appropriately).
RFC3810, section 9.11. only changed that default interval into 1 second (in
contrast to the older RFC2710). Therefore, do not update the URI sysctl
provided interval value when receiving an MLDv2 query, only pass that max
delay as mentioned in section 5. along to mld_gq_start_timer(). This
behaviour seems to be the case since the initial implementation of MLDv2.
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
[ Sending to net-next to let this linger a bit here first, seems to be
the case like this since initial MLDv2. ]
net/ipv6/mcast.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 3d0e8fc..2a4d2b1 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -994,9 +994,9 @@ bool ipv6_chk_mcast_addr(struct net_device *dev, const struct in6_addr *group,
return rv;
}
-static void mld_gq_start_timer(struct inet6_dev *idev)
+static void mld_gq_start_timer(struct inet6_dev *idev, unsigned long delay)
{
- unsigned long tv = prandom_u32() % idev->mc_uri;
+ unsigned long tv = prandom_u32() % delay;
idev->mc_gq_running = 1;
if (!mod_timer(&idev->mc_gq_timer, jiffies+tv+2))
@@ -1274,8 +1274,6 @@ static int mld_process_v2(struct inet6_dev *idev, struct mld2_query *mld,
mld_update_qi(idev, mld);
mld_update_qri(idev, mld);
- idev->mc_uri = *max_delay;
-
return 0;
}
@@ -1345,7 +1343,7 @@ int igmp6_event_query(struct sk_buff *skb)
if (mlh2->mld2q_nsrcs)
return -EINVAL; /* no sources allowed */
- mld_gq_start_timer(idev);
+ mld_gq_start_timer(idev, max_delay);
return 0;
}
/* mark sources to include, if group & source-specific */
--
1.7.11.7
^ permalink raw reply related
* [PATCH net-next 0/3] mld updates, part 2
From: Daniel Borkmann @ 2014-09-23 7:03 UTC (permalink / raw)
To: davem; +Cc: hannes, netdev
Daniel Borkmann (3):
ipv6: mld: rename mc_maxdelay into mc_uri
ipv6: mld: do not overwrite uri when receiving an mldv2 query
ipv6: mld: remove duplicate code from mld_update_qri
include/net/if_inet6.h | 2 +-
net/ipv6/mcast.c | 31 ++++++++++++-------------------
2 files changed, 13 insertions(+), 20 deletions(-)
--
1.7.11.7
^ permalink raw reply
* [PATCH net-next 3/3] ipv6: mld: remove duplicate code from mld_update_qri
From: Daniel Borkmann @ 2014-09-23 7:03 UTC (permalink / raw)
To: davem; +Cc: hannes, netdev
In-Reply-To: <1411455828-5196-1-git-send-email-dborkman@redhat.com>
The QRI (Query Response Interval) from RFC3810, section 9.3. is the same
as we calculate anyway earlier. Therefore, we can just remove that code
and simply reuse the value of max_delay.
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
net/ipv6/mcast.c | 17 ++++++-----------
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 2a4d2b1..e4139e5 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -1226,16 +1226,6 @@ static void mld_update_qi(struct inet6_dev *idev,
idev->mc_qi = mc_qqi * HZ;
}
-static void mld_update_qri(struct inet6_dev *idev,
- const struct mld2_query *mlh2)
-{
- /* RFC3810, relevant sections:
- * - 5.1.3. Maximum Response Code
- * - 9.3. Query Response Interval
- */
- idev->mc_qri = msecs_to_jiffies(mldv2_mrc(mlh2));
-}
-
static int mld_process_v1(struct inet6_dev *idev, struct mld_msg *mld,
unsigned long *max_delay)
{
@@ -1272,7 +1262,12 @@ static int mld_process_v2(struct inet6_dev *idev, struct mld2_query *mld,
mld_update_qrv(idev, mld);
mld_update_qi(idev, mld);
- mld_update_qri(idev, mld);
+
+ /* RFC3810, relevant sections:
+ * - 5.1.3. Maximum Response Code
+ * - 9.3. Query Response Interval
+ */
+ idev->mc_qri = *max_delay;
return 0;
}
--
1.7.11.7
^ permalink raw reply related
* Re: [patch net-next v2 8/9] switchdev: introduce Netlink API
From: Thomas Graf @ 2014-09-23 7:19 UTC (permalink / raw)
To: John Fastabend
Cc: Tom Herbert, Alexei Starovoitov, Jiri Pirko, John Fastabend,
Jamal Hadi Salim, netdev@vger.kernel.org, David S. Miller,
Neil Horman, Andy Gospodarek, Daniel Borkmann, Or Gerlitz,
Jesse Gross, Pravin Shelar, Andy Zhou, Ben Hutchings,
Stephen Hemminger, Jeff Kirsher, Vladislav Yasevich, Cong Wang,
Eric Dumazet, Scott Feldman, Florian Fainelli, Roop
In-Reply-To: <5420CEAC.2070104@gmail.com>
On 09/22/14 at 06:36pm, John Fastabend wrote:
> n-tuple has some deficiencies,
>
> - its not possible to get the capabilities to learn what
> fields are supported by the device, what actions, etc.
>
> - its ioctl based so we have to poll the device
>
> - only supports a single table, where we have devices with
> multiple tables
>
> - sort of the same as above but it doesn't allow creating new
> tables or destroying old tables.
OK, I understand where Tom was going. Given we add feature detection
capabilities this could be used to identify the guest for fixed length
encap. I still assume HW won't be able to match on the inner header
for any variable lengh encap with metadata packet unless it can
actually parse the encap. I hope I didn't bring encap format to this
thread at this very moment ;-)
^ permalink raw reply
* Re: [PATCH net-next 3/3] tcp: better TCP_SKB_CB layout to reduce cache line misses
From: Christoph Paasch @ 2014-09-23 7:20 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S. Miller, netdev, Yuchung Cheng, Neal Cardwell
In-Reply-To: <1411429844-11099-4-git-send-email-edumazet@google.com>
Hello Eric,
On 22/09/14 - 16:50:44, Eric Dumazet wrote:
> TCP maintains lists of skb in write queue, and in receive queues
> (in order and out of order queues)
>
> Scanning these lists both in input and output path usually requires
> access to skb->next, TCP_SKB_CB(skb)->seq, and TCP_SKB_CB(skb)->end_seq
>
> These fields are currently in two different cache lines, meaning we
> waste lot of memory bandwidth when these queues are big and flows
> have either packet drops or packet reorders.
>
> We can move TCP_SKB_CB(skb)->header at the end of TCP_SKB_CB, because
> this header is not used in fast path. This allows TCP to search much faster
> in the skb lists.
>
> Even with regular flows, we save one cache line miss in fast path.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---
> include/net/tcp.h | 12 ++++++------
> net/ipv4/tcp_ipv4.c | 7 +++++++
> net/ipv6/tcp_ipv6.c | 7 +++++++
> 3 files changed, 20 insertions(+), 6 deletions(-)
doesn't also the tx-path relies on IPCB(skb) being memset to 0 (e.g., in
xfrm4_output or ip_options_build)
Or is it somewhere already explicitly memset to 0? (didn't found this place
in the code though)
Cheers,
Christoph
^ permalink raw reply
* Re: [RFC PATCH linux-next] et131x: Promote staging et131x driver to drivers/net
From: Tobias Klauser @ 2014-09-23 7:22 UTC (permalink / raw)
To: Mark Einon; +Cc: devel, gregkh, davem, linux-kernel, netdev
In-Reply-To: <1411421283-25073-1-git-send-email-mark.einon@gmail.com>
On 2014-09-22 at 23:28:03 +0200, Mark Einon <mark.einon@gmail.com> wrote:
> This patch moves the et131x gigabit ethernet driver from drivers/staging
> to drivers/net/ethernet/agere.
>
> There are no known issues at this time.
>
> Signed-off-by: Mark Einon <mark.einon@gmail.com>
> ---
>
> This patch will only apply once the last few pending changes
> make their way from staging-next to linux-next, but posting
> now in the hope that feedback can be given and this change can
> make it in before the next merge window.
>
> drivers/net/ethernet/Kconfig | 1 +
> drivers/net/ethernet/Makefile | 1 +
> drivers/net/ethernet/agere/Kconfig | 31 +
> drivers/net/ethernet/agere/Makefile | 5 +
> drivers/net/ethernet/agere/et131x.c | 4412 +++++++++++++++++++++++++++++++++++
> drivers/net/ethernet/agere/et131x.h | 1533 ++++++++++++
> drivers/staging/Kconfig | 2 -
> drivers/staging/Makefile | 1 -
> drivers/staging/et131x/Kconfig | 10 -
> drivers/staging/et131x/Makefile | 5 -
> drivers/staging/et131x/README | 15 -
> drivers/staging/et131x/et131x.c | 4412 -----------------------------------
> drivers/staging/et131x/et131x.h | 1533 ------------
> 13 files changed, 5983 insertions(+), 5978 deletions(-)
> create mode 100644 drivers/net/ethernet/agere/Kconfig
> create mode 100644 drivers/net/ethernet/agere/Makefile
> create mode 100644 drivers/net/ethernet/agere/et131x.c
> create mode 100644 drivers/net/ethernet/agere/et131x.h
> delete mode 100644 drivers/staging/et131x/Kconfig
> delete mode 100644 drivers/staging/et131x/Makefile
> delete mode 100644 drivers/staging/et131x/README
> delete mode 100644 drivers/staging/et131x/et131x.c
> delete mode 100644 drivers/staging/et131x/et131x.h
[...]
> diff --git a/drivers/net/ethernet/agere/et131x.c b/drivers/net/ethernet/agere/et131x.c
> new file mode 100644
> index 0000000..93afd61
> --- /dev/null
> +++ b/drivers/net/ethernet/agere/et131x.c
> @@ -0,0 +1,4412 @@
[...]
> +/* et131x_rx_dma_memory_alloc
> + *
> + * Allocates Free buffer ring 1 for sure, free buffer ring 0 if required,
> + * and the Packet Status Ring.
> + */
> +static int et131x_rx_dma_memory_alloc(struct et131x_adapter *adapter)
> +{
> + u8 id;
> + u32 i, j;
> + u32 bufsize;
> + u32 psr_size;
> + u32 fbr_chunksize;
> + struct rx_ring *rx_ring = &adapter->rx_ring;
> + struct fbr_lookup *fbr;
> +
> + /* Alloc memory for the lookup table */
> + rx_ring->fbr[0] = kmalloc(sizeof(*fbr), GFP_KERNEL);
> + if (rx_ring->fbr[0] == NULL)
> + return -ENOMEM;
> + rx_ring->fbr[1] = kmalloc(sizeof(*fbr), GFP_KERNEL);
> + if (rx_ring->fbr[1] == NULL)
> + return -ENOMEM;
If you fail here, et131x_rx_dma_memory_free() will be called by
et131x_adapter_memory_free() in the error handling path which in turn
will access the members of the already allocated rx_ring->fbr[0] (e.g.
->buffsize). Since it is allocated with kmalloc() these members contain
arbitrary values and cause problems in et131x_rx_dma_memory_free(). I'
suggest to do the cleanup (i.e. free rx_ring->fbr[0] directly here and
not call et131x_rx_dma_memory_free() in the error handling path. You
might want to check that memory allocation failures are properly handled
in the rest of the driver as well. There are multiple other cases where
et131x_adapter_memory_free() is called on partially
allocated/initialized memory.
> +
> + /* The first thing we will do is configure the sizes of the buffer
> + * rings. These will change based on jumbo packet support. Larger
> + * jumbo packets increases the size of each entry in FBR0, and the
> + * number of entries in FBR0, while at the same time decreasing the
> + * number of entries in FBR1.
> + *
> + * FBR1 holds "large" frames, FBR0 holds "small" frames. If FBR1
> + * entries are huge in order to accommodate a "jumbo" frame, then it
> + * will have less entries. Conversely, FBR1 will now be relied upon
> + * to carry more "normal" frames, thus it's entry size also increases
> + * and the number of entries goes up too (since it now carries
> + * "small" + "regular" packets.
> + *
> + * In this scheme, we try to maintain 512 entries between the two
> + * rings. Also, FBR1 remains a constant size - when it's size doubles
> + * the number of entries halves. FBR0 increases in size, however.
> + */
> + if (adapter->registry_jumbo_packet < 2048) {
> + rx_ring->fbr[0]->buffsize = 256;
> + rx_ring->fbr[0]->num_entries = 512;
> + rx_ring->fbr[1]->buffsize = 2048;
> + rx_ring->fbr[1]->num_entries = 512;
> + } else if (adapter->registry_jumbo_packet < 4096) {
> + rx_ring->fbr[0]->buffsize = 512;
> + rx_ring->fbr[0]->num_entries = 1024;
> + rx_ring->fbr[1]->buffsize = 4096;
> + rx_ring->fbr[1]->num_entries = 512;
> + } else {
> + rx_ring->fbr[0]->buffsize = 1024;
> + rx_ring->fbr[0]->num_entries = 768;
> + rx_ring->fbr[1]->buffsize = 16384;
> + rx_ring->fbr[1]->num_entries = 128;
> + }
> +
> + rx_ring->psr_entries = rx_ring->fbr[0]->num_entries +
> + rx_ring->fbr[1]->num_entries;
> +
> + for (id = 0; id < NUM_FBRS; id++) {
> + fbr = rx_ring->fbr[id];
> + /* Allocate an area of memory for Free Buffer Ring */
> + bufsize = sizeof(struct fbr_desc) * fbr->num_entries;
> + fbr->ring_virtaddr = dma_alloc_coherent(&adapter->pdev->dev,
> + bufsize,
> + &fbr->ring_physaddr,
> + GFP_KERNEL);
> + if (!fbr->ring_virtaddr) {
> + dev_err(&adapter->pdev->dev,
> + "Cannot alloc memory for Free Buffer Ring %d\n",
> + id);
> + return -ENOMEM;
> + }
> + }
> +
> + for (id = 0; id < NUM_FBRS; id++) {
> + fbr = rx_ring->fbr[id];
> + fbr_chunksize = (FBR_CHUNKS * fbr->buffsize);
> +
> + for (i = 0; i < fbr->num_entries / FBR_CHUNKS; i++) {
> + dma_addr_t fbr_physaddr;
> +
> + fbr->mem_virtaddrs[i] = dma_alloc_coherent(
> + &adapter->pdev->dev, fbr_chunksize,
> + &fbr->mem_physaddrs[i],
> + GFP_KERNEL);
> +
> + if (!fbr->mem_virtaddrs[i]) {
> + dev_err(&adapter->pdev->dev,
> + "Could not alloc memory\n");
> + return -ENOMEM;
> + }
> +
> + /* See NOTE in "Save Physical Address" comment above */
> + fbr_physaddr = fbr->mem_physaddrs[i];
> +
> + for (j = 0; j < FBR_CHUNKS; j++) {
> + u32 k = (i * FBR_CHUNKS) + j;
> +
> + /* Save the Virtual address of this index for
> + * quick access later
> + */
> + fbr->virt[k] = (u8 *)fbr->mem_virtaddrs[i] +
> + (j * fbr->buffsize);
> +
> + /* now store the physical address in the
> + * descriptor so the device can access it
> + */
> + fbr->bus_high[k] = upper_32_bits(fbr_physaddr);
> + fbr->bus_low[k] = lower_32_bits(fbr_physaddr);
> + fbr_physaddr += fbr->buffsize;
> + }
> + }
> + }
> +
> + /* Allocate an area of memory for FIFO of Packet Status ring entries */
> + psr_size = sizeof(struct pkt_stat_desc) * rx_ring->psr_entries;
> +
> + rx_ring->ps_ring_virtaddr = dma_alloc_coherent(&adapter->pdev->dev,
> + psr_size,
> + &rx_ring->ps_ring_physaddr,
> + GFP_KERNEL);
> +
> + if (!rx_ring->ps_ring_virtaddr) {
> + dev_err(&adapter->pdev->dev,
> + "Cannot alloc memory for Packet Status Ring\n");
> + return -ENOMEM;
> + }
> +
> + /* NOTE : dma_alloc_coherent(), used above to alloc DMA regions,
> + * ALWAYS returns SAC (32-bit) addresses. If DAC (64-bit) addresses
> + * are ever returned, make sure the high part is retrieved here before
> + * storing the adjusted address.
> + */
> +
> + /* Allocate an area of memory for writeback of status information */
> + rx_ring->rx_status_block = dma_alloc_coherent(&adapter->pdev->dev,
> + sizeof(struct rx_status_block),
> + &rx_ring->rx_status_bus,
> + GFP_KERNEL);
> + if (!rx_ring->rx_status_block) {
> + dev_err(&adapter->pdev->dev,
> + "Cannot alloc memory for Status Block\n");
> + return -ENOMEM;
> + }
> + rx_ring->num_rfd = NIC_DEFAULT_NUM_RFD;
> +
> + /* The RFDs are going to be put on lists later on, so initialize the
> + * lists now.
> + */
> + INIT_LIST_HEAD(&rx_ring->recv_list);
> + return 0;
> +}
> +
> +/* et131x_rx_dma_memory_free - Free all memory allocated within this module */
> +static void et131x_rx_dma_memory_free(struct et131x_adapter *adapter)
> +{
> + u8 id;
> + u32 ii;
> + u32 bufsize;
> + u32 psr_size;
> + struct rfd *rfd;
> + struct rx_ring *rx_ring = &adapter->rx_ring;
> + struct fbr_lookup *fbr;
> +
> + /* Free RFDs and associated packet descriptors */
> + WARN_ON(rx_ring->num_ready_recv != rx_ring->num_rfd);
> +
> + while (!list_empty(&rx_ring->recv_list)) {
> + rfd = list_entry(rx_ring->recv_list.next,
> + struct rfd, list_node);
> +
> + list_del(&rfd->list_node);
> + rfd->skb = NULL;
> + kfree(rfd);
> + }
> +
> + /* Free Free Buffer Rings */
> + for (id = 0; id < NUM_FBRS; id++) {
> + fbr = rx_ring->fbr[id];
> +
> + if (!fbr || !fbr->ring_virtaddr)
> + continue;
> +
> + /* First the packet memory */
> + for (ii = 0; ii < fbr->num_entries / FBR_CHUNKS; ii++) {
> + if (fbr->mem_virtaddrs[ii]) {
> + bufsize = fbr->buffsize * FBR_CHUNKS;
> +
> + dma_free_coherent(&adapter->pdev->dev,
> + bufsize,
> + fbr->mem_virtaddrs[ii],
> + fbr->mem_physaddrs[ii]);
> +
> + fbr->mem_virtaddrs[ii] = NULL;
> + }
> + }
> +
> + bufsize = sizeof(struct fbr_desc) * fbr->num_entries;
> +
> + dma_free_coherent(&adapter->pdev->dev,
> + bufsize,
> + fbr->ring_virtaddr,
> + fbr->ring_physaddr);
> +
> + fbr->ring_virtaddr = NULL;
> + }
> +
> + /* Free Packet Status Ring */
> + if (rx_ring->ps_ring_virtaddr) {
> + psr_size = sizeof(struct pkt_stat_desc) * rx_ring->psr_entries;
> +
> + dma_free_coherent(&adapter->pdev->dev, psr_size,
> + rx_ring->ps_ring_virtaddr,
> + rx_ring->ps_ring_physaddr);
> +
> + rx_ring->ps_ring_virtaddr = NULL;
> + }
> +
> + /* Free area of memory for the writeback of status information */
> + if (rx_ring->rx_status_block) {
> + dma_free_coherent(&adapter->pdev->dev,
> + sizeof(struct rx_status_block),
> + rx_ring->rx_status_block,
> + rx_ring->rx_status_bus);
> + rx_ring->rx_status_block = NULL;
> + }
> +
> + /* Free the FBR Lookup Table */
> + kfree(rx_ring->fbr[0]);
> + kfree(rx_ring->fbr[1]);
> +
> + /* Reset Counters */
> + rx_ring->num_ready_recv = 0;
> +}
[...]
> +/* et131x_adapter_memory_free - Free all memory allocated for use by Tx & Rx */
> +static void et131x_adapter_memory_free(struct et131x_adapter *adapter)
> +{
> + et131x_tx_dma_memory_free(adapter);
> + et131x_rx_dma_memory_free(adapter);
> +}
> +
> +/* et131x_adapter_memory_alloc
> + * Allocate all the memory blocks for send, receive and others.
> + */
> +static int et131x_adapter_memory_alloc(struct et131x_adapter *adapter)
> +{
> + int status;
> +
> + /* Allocate memory for the Tx Ring */
> + status = et131x_tx_dma_memory_alloc(adapter);
> + if (status) {
> + dev_err(&adapter->pdev->dev,
> + "et131x_tx_dma_memory_alloc FAILED\n");
> + et131x_tx_dma_memory_free(adapter);
> + return status;
> + }
> + /* Receive buffer memory allocation */
> + status = et131x_rx_dma_memory_alloc(adapter);
> + if (status) {
> + dev_err(&adapter->pdev->dev,
> + "et131x_rx_dma_memory_alloc FAILED\n");
> + et131x_adapter_memory_free(adapter);
> + return status;
> + }
> +
> + /* Init receive data structures */
> + status = et131x_init_recv(adapter);
> + if (status) {
> + dev_err(&adapter->pdev->dev, "et131x_init_recv FAILED\n");
> + et131x_adapter_memory_free(adapter);
> + }
> + return status;
> +}
[...]
> +#ifdef CONFIG_PM_SLEEP
> +static int et131x_suspend(struct device *dev)
> +{
> + struct pci_dev *pdev = to_pci_dev(dev);
> + struct net_device *netdev = pci_get_drvdata(pdev);
> +
> + if (netif_running(netdev)) {
> + netif_device_detach(netdev);
> + et131x_down(netdev);
> + pci_save_state(pdev);
> + }
> +
> + return 0;
> +}
> +
> +static int et131x_resume(struct device *dev)
> +{
> + struct pci_dev *pdev = to_pci_dev(dev);
> + struct net_device *netdev = pci_get_drvdata(pdev);
> +
> + if (netif_running(netdev)) {
> + pci_restore_state(pdev);
> + et131x_up(netdev);
> + netif_device_attach(netdev);
> + }
> +
> + return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(et131x_pm_ops, et131x_suspend, et131x_resume);
> +#define ET131X_PM_OPS (&et131x_pm_ops)
> +#else
> +#define ET131X_PM_OPS NULL
> +#endif
No need for the #define here, just assigne et131x_pm_ops to .driver.pm
directly, its members will be NULL and thus never called. Also, you can
make et131x_pm_ops const.
> +
> +/* et131x_isr - The Interrupt Service Routine for the driver.
> + * @irq: the IRQ on which the interrupt was received.
> + * @dev_id: device-specific info (here a pointer to a net_device struct)
> + *
> + * Returns a value indicating if the interrupt was handled.
> + */
> +static irqreturn_t et131x_isr(int irq, void *dev_id)
> +{
> + bool handled = true;
> + bool enable_interrupts = true;
> + struct net_device *netdev = (struct net_device *)dev_id;
No need to cast a void *.
> + struct et131x_adapter *adapter = netdev_priv(netdev);
> + struct address_map __iomem *iomem = adapter->regs;
> + struct rx_ring *rx_ring = &adapter->rx_ring;
> + struct tx_ring *tx_ring = &adapter->tx_ring;
> + u32 status;
> +
> + if (!netif_device_present(netdev)) {
> + handled = false;
> + enable_interrupts = false;
> + goto out;
> + }
> +
> + /* If the adapter is in low power state, then it should not
> + * recognize any interrupt
> + */
> +
> + /* Disable Device Interrupts */
> + et131x_disable_interrupts(adapter);
> +
> + /* Get a copy of the value in the interrupt status register
> + * so we can process the interrupting section
> + */
> + status = readl(&adapter->regs->global.int_status);
> +
> + if (adapter->flow == FLOW_TXONLY || adapter->flow == FLOW_BOTH)
> + status &= ~INT_MASK_ENABLE;
> + else
> + status &= ~INT_MASK_ENABLE_NO_FLOW;
> +
> + /* Make sure this is our interrupt */
> + if (!status) {
> + handled = false;
> + et131x_enable_interrupts(adapter);
> + goto out;
> + }
> +
> + /* This is our interrupt, so process accordingly */
> + if (status & ET_INTR_WATCHDOG) {
> + struct tcb *tcb = tx_ring->send_head;
> +
> + if (tcb)
> + if (++tcb->stale > 1)
> + status |= ET_INTR_TXDMA_ISR;
> +
> + if (rx_ring->unfinished_receives)
> + status |= ET_INTR_RXDMA_XFR_DONE;
> + else if (tcb == NULL)
> + writel(0, &adapter->regs->global.watchdog_timer);
> +
> + status &= ~ET_INTR_WATCHDOG;
> + }
> +
> + if (status & (ET_INTR_RXDMA_XFR_DONE | ET_INTR_TXDMA_ISR)) {
> + enable_interrupts = false;
> + napi_schedule(&adapter->napi);
> + }
> +
> + status &= ~(ET_INTR_TXDMA_ISR | ET_INTR_RXDMA_XFR_DONE);
> +
> + if (!status)
> + goto out;
> +
> + /* Handle the TXDMA Error interrupt */
> + if (status & ET_INTR_TXDMA_ERR) {
> + /* Following read also clears the register (COR) */
> + u32 txdma_err = readl(&iomem->txdma.tx_dma_error);
> +
> + dev_warn(&adapter->pdev->dev,
> + "TXDMA_ERR interrupt, error = %d\n",
> + txdma_err);
> + }
> +
> + /* Handle Free Buffer Ring 0 and 1 Low interrupt */
> + if (status & (ET_INTR_RXDMA_FB_R0_LOW | ET_INTR_RXDMA_FB_R1_LOW)) {
> + /* This indicates the number of unused buffers in RXDMA free
> + * buffer ring 0 is <= the limit you programmed. Free buffer
> + * resources need to be returned. Free buffers are consumed as
> + * packets are passed from the network to the host. The host
> + * becomes aware of the packets from the contents of the packet
> + * status ring. This ring is queried when the packet done
> + * interrupt occurs. Packets are then passed to the OS. When
> + * the OS is done with the packets the resources can be
> + * returned to the ET1310 for re-use. This interrupt is one
> + * method of returning resources.
> + */
> +
> + /* If the user has flow control on, then we will
> + * send a pause packet, otherwise just exit
> + */
> + if (adapter->flow == FLOW_TXONLY || adapter->flow == FLOW_BOTH) {
> + u32 pm_csr;
> +
> + /* Tell the device to send a pause packet via the back
> + * pressure register (bp req and bp xon/xoff)
> + */
> + pm_csr = readl(&iomem->global.pm_csr);
> + if (!et1310_in_phy_coma(adapter))
> + writel(3, &iomem->txmac.bp_ctrl);
> + }
> + }
> +
> + /* Handle Packet Status Ring Low Interrupt */
> + if (status & ET_INTR_RXDMA_STAT_LOW) {
> + /* Same idea as with the two Free Buffer Rings. Packets going
> + * from the network to the host each consume a free buffer
> + * resource and a packet status resource. These resources are
> + * passed to the OS. When the OS is done with the resources,
> + * they need to be returned to the ET1310. This is one method
> + * of returning the resources.
> + */
> + }
> +
> + /* Handle RXDMA Error Interrupt */
> + if (status & ET_INTR_RXDMA_ERR) {
> + /* The rxdma_error interrupt is sent when a time-out on a
> + * request issued by the JAGCore has occurred or a completion is
> + * returned with an un-successful status. In both cases the
> + * request is considered complete. The JAGCore will
> + * automatically re-try the request in question. Normally
> + * information on events like these are sent to the host using
> + * the "Advanced Error Reporting" capability. This interrupt is
> + * another way of getting similar information. The only thing
> + * required is to clear the interrupt by reading the ISR in the
> + * global resources. The JAGCore will do a re-try on the
> + * request. Normally you should never see this interrupt. If
> + * you start to see this interrupt occurring frequently then
> + * something bad has occurred. A reset might be the thing to do.
> + */
> + /* TRAP();*/
> +
> + dev_warn(&adapter->pdev->dev,
> + "RxDMA_ERR interrupt, error %x\n",
> + readl(&iomem->txmac.tx_test));
> + }
> +
> + /* Handle the Wake on LAN Event */
> + if (status & ET_INTR_WOL) {
> + /* This is a secondary interrupt for wake on LAN. The driver
> + * should never see this, if it does, something serious is
> + * wrong. We will TRAP the message when we are in DBG mode,
> + * otherwise we will ignore it.
> + */
> + dev_err(&adapter->pdev->dev, "WAKE_ON_LAN interrupt\n");
> + }
> +
> + /* Let's move on to the TxMac */
> + if (status & ET_INTR_TXMAC) {
> + u32 err = readl(&iomem->txmac.err);
> +
> + /* When any of the errors occur and TXMAC generates an
> + * interrupt to report these errors, it usually means that
> + * TXMAC has detected an error in the data stream retrieved
> + * from the on-chip Tx Q. All of these errors are catastrophic
> + * and TXMAC won't be able to recover data when these errors
> + * occur. In a nutshell, the whole Tx path will have to be reset
> + * and re-configured afterwards.
> + */
> + dev_warn(&adapter->pdev->dev,
> + "TXMAC interrupt, error 0x%08x\n",
> + err);
> +
> + /* If we are debugging, we want to see this error, otherwise we
> + * just want the device to be reset and continue
> + */
> + }
> +
> + /* Handle RXMAC Interrupt */
> + if (status & ET_INTR_RXMAC) {
> + /* These interrupts are catastrophic to the device, what we need
> + * to do is disable the interrupts and set the flag to cause us
> + * to reset so we can solve this issue.
> + */
> + /* MP_SET_FLAG( adapter, FMP_ADAPTER_HARDWARE_ERROR); */
> +
> + dev_warn(&adapter->pdev->dev,
> + "RXMAC interrupt, error 0x%08x. Requesting reset\n",
> + readl(&iomem->rxmac.err_reg));
> +
> + dev_warn(&adapter->pdev->dev,
> + "Enable 0x%08x, Diag 0x%08x\n",
> + readl(&iomem->rxmac.ctrl),
> + readl(&iomem->rxmac.rxq_diag));
> +
> + /* If we are debugging, we want to see this error, otherwise we
> + * just want the device to be reset and continue
> + */
> + }
> +
> + /* Handle MAC_STAT Interrupt */
> + if (status & ET_INTR_MAC_STAT) {
> + /* This means at least one of the un-masked counters in the
> + * MAC_STAT block has rolled over. Use this to maintain the top,
> + * software managed bits of the counter(s).
> + */
> + et1310_handle_macstat_interrupt(adapter);
> + }
> +
> + /* Handle SLV Timeout Interrupt */
> + if (status & ET_INTR_SLV_TIMEOUT) {
> + /* This means a timeout has occurred on a read or write request
> + * to one of the JAGCore registers. The Global Resources block
> + * has terminated the request and on a read request, returned a
> + * "fake" value. The most likely reasons are: Bad Address or the
> + * addressed module is in a power-down state and can't respond.
> + */
> + }
> +
> +out:
> + if (enable_interrupts)
> + et131x_enable_interrupts(adapter);
> +
> + return IRQ_RETVAL(handled);
> +}
[...]
> +static const struct pci_device_id et131x_pci_table[] = {
> + { PCI_VDEVICE(ATT, ET131X_PCI_DEVICE_ID_GIG), 0UL},
> + { PCI_VDEVICE(ATT, ET131X_PCI_DEVICE_ID_FAST), 0UL},
> + {0,}
Nit: Space after opening curly brace.
^ permalink raw reply
* Re: [PATCH net-next 3/3] tcp: better TCP_SKB_CB layout to reduce cache line misses
From: Christoph Paasch @ 2014-09-23 7:36 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S. Miller, netdev, Yuchung Cheng, Neal Cardwell
In-Reply-To: <20140923072016.GL3291@cpaasch-mac>
On 23/09/14 - 09:20:16, Christoph Paasch wrote:
> Hello Eric,
>
> On 22/09/14 - 16:50:44, Eric Dumazet wrote:
> > TCP maintains lists of skb in write queue, and in receive queues
> > (in order and out of order queues)
> >
> > Scanning these lists both in input and output path usually requires
> > access to skb->next, TCP_SKB_CB(skb)->seq, and TCP_SKB_CB(skb)->end_seq
> >
> > These fields are currently in two different cache lines, meaning we
> > waste lot of memory bandwidth when these queues are big and flows
> > have either packet drops or packet reorders.
> >
> > We can move TCP_SKB_CB(skb)->header at the end of TCP_SKB_CB, because
> > this header is not used in fast path. This allows TCP to search much faster
> > in the skb lists.
> >
> > Even with regular flows, we save one cache line miss in fast path.
> >
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > ---
> > include/net/tcp.h | 12 ++++++------
> > net/ipv4/tcp_ipv4.c | 7 +++++++
> > net/ipv6/tcp_ipv6.c | 7 +++++++
> > 3 files changed, 20 insertions(+), 6 deletions(-)
>
> doesn't also the tx-path relies on IPCB(skb) being memset to 0 (e.g., in
> xfrm4_output or ip_options_build)
Crap, forget about ip_options_build. It is memcpy'ing into IPCB... Confused
src with dest... :-)
Nevertheless, wouldn't there be a problem in xfrm4_output?
Cheers,
Christoph
^ permalink raw reply
* Re: [PATCHv3 1/1] bluetooth: Check for SCO type before setting retransmission effort
From: Marcel Holtmann @ 2014-09-23 7:44 UTC (permalink / raw)
To: Bernhard Thaler
Cc: Gustavo F. Padovan, Johan Hedberg, David S. Miller,
linux-bluetooth, netdev, linux-kernel
In-Reply-To: <1411451964-4025-1-git-send-email-bernhard.thaler@r-it.at>
Hi Bernhard,
> SCO connection cannot be setup to devices that do not support retransmission.
> Patch based on http://permalink.gmane.org/gmane.linux.bluez.kernel/7779 and
> adapted for this kernel version.
> Code changed to check SCO/eSCO type before setting retransmission effort
> and max. latency. The purpose of the patch is to support older devices not
> capable of eSCO.
>
> Tested on Blackberry 655+ headset which does not support retransmission.
> Credits go to Alexander Sommerhuber.
>
> Signed-off-by: Bernhard Thaler <bernhard.thaler@r-it.at>
> ---
> net/bluetooth/hci_conn.c | 22 +++++++++++++++++-----
> 1 file changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index faff624..32dcde1 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -38,7 +38,7 @@ struct sco_param {
> u16 max_latency;
> };
>
> -static const struct sco_param sco_param_cvsd[] = {
> +static const struct sco_param esco_param_cvsd[] = {
> { EDR_ESCO_MASK & ~ESCO_2EV3, 0x000a }, /* S3 */
> { EDR_ESCO_MASK & ~ESCO_2EV3, 0x0007 }, /* S2 */
> { EDR_ESCO_MASK | ESCO_EV3, 0x0007 }, /* S1 */
> @@ -46,6 +46,11 @@ static const struct sco_param sco_param_cvsd[] = {
> { EDR_ESCO_MASK | ESCO_HV1, 0xffff }, /* D0 */
> };
>
> +static const struct sco_param sco_param_cvsd[] = {
> + { EDR_ESCO_MASK | ESCO_HV3, 0xffff }, /* D1 */
> + { EDR_ESCO_MASK | ESCO_HV1, 0xffff }, /* D0 */
> +};
> +
> static const struct sco_param sco_param_wideband[] = {
> { EDR_ESCO_MASK & ~ESCO_2EV3, 0x000d }, /* T2 */
> { EDR_ESCO_MASK | ESCO_EV3, 0x0008 }, /* T1 */
> @@ -194,10 +199,17 @@ bool hci_setup_sync(struct hci_conn *conn, __u16 handle)
> param = &sco_param_wideband[conn->attempt - 1];
> break;
> case SCO_AIRMODE_CVSD:
> - if (conn->attempt > ARRAY_SIZE(sco_param_cvsd))
> - return false;
> - cp.retrans_effort = 0x01;
> - param = &sco_param_cvsd[conn->attempt - 1];
> + if (!lmp_esco_capable(conn->link)) {
please change this statement around to check if eSCO support exists and not if it does not exist. The negation is pointless if you have an else branch anyway.
> + if ((conn->attempt) > ARRAY_SIZE(sco_param_cvsd))
The extra (conn->attempt) is not needed. Just do (conn->attempt > ARRAY_SIZE(..))
> + return false;
> + cp.retrans_effort = 0xff;
> + param = &sco_param_cvsd[conn->attempt - 1];
> + } else {
> + if (conn->attempt > ARRAY_SIZE(esco_param_cvsd))
> + return false;
> + cp.retrans_effort = 0x01;
> + param = &esco_param_cvsd[conn->attempt - 1];
> + }
> break;
> default:
> return false;
The rest looks fine.
Regards
Marcel
^ permalink raw reply
* [PATCH v1 4/4] net: fec: free resource after phy probe failed
From: Fugang Duan @ 2014-09-23 7:40 UTC (permalink / raw)
To: davem; +Cc: netdev, b20596, b38611
In-Reply-To: <1411458058-18934-1-git-send-email-b38611@freescale.com>
Free memory and disable all related clocks when there has no phy
connection or phy probe failed.
Signed-off-by: Fugang Duan <B38611@freescale.com>
---
drivers/net/ethernet/freescale/fec_main.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 3ce3567..2b16ead 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -2675,6 +2675,8 @@ fec_enet_open(struct net_device *ndev)
ret = fec_enet_mii_probe(ndev);
if (ret) {
fec_enet_free_buffers(ndev);
+ fec_enet_clk_enable(ndev, false);
+ pinctrl_pm_select_sleep_state(&fep->pdev->dev);
return ret;
}
--
1.7.8
^ permalink raw reply related
* [PATCH v1 1/4] net: fec: Add Ftype to BD to distiguish three tx queues for AVB
From: Fugang Duan @ 2014-09-23 7:40 UTC (permalink / raw)
To: davem; +Cc: netdev, b20596, b38611
In-Reply-To: <1411458058-18934-1-git-send-email-b38611@freescale.com>
The current driver loss Ftype field init for BD, which cause tx
queue #1 and #2 cannot work well.
Add Ftype field to BD to distiguish three queues for AVB:
0 -> Best Effort
1 -> ClassA
2 -> ClassB
Signed-off-by: Fugang Duan <B38611@freescale.com>
---
drivers/net/ethernet/freescale/fec.h | 1 +
drivers/net/ethernet/freescale/fec_main.c | 11 +++++++++++
2 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index 26fb1de..354a309 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -308,6 +308,7 @@ struct bufdesc_ex {
#define RCMR_CMP_2 (RCMR_CMP_CFG(4, 0) | RCMR_CMP_CFG(5, 1) | \
RCMR_CMP_CFG(6, 2) | RCMR_CMP_CFG(7, 3))
#define RCMR_CMP(X) ((X == 1) ? RCMR_CMP_1 : RCMR_CMP_2)
+#define FEC_TX_BD_FTYPE(X) ((X & 0xF) << 20)
/* The number of Tx and Rx buffers. These are allocated from the page
* pool. The code may assume these are power of two, so it it best
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 3a4ec0f..a0f21ce 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -426,6 +426,8 @@ fec_enet_txq_submit_frag_skb(struct fec_enet_priv_tx_q *txq,
}
if (fep->bufdesc_ex) {
+ if (id_entry->driver_data & FEC_QUIRK_HAS_AVB)
+ estatus |= FEC_TX_BD_FTYPE(queue);
if (skb->ip_summed == CHECKSUM_PARTIAL)
estatus |= BD_ENET_TX_PINS | BD_ENET_TX_IINS;
ebdp->cbd_bdu = 0;
@@ -555,6 +557,9 @@ static int fec_enet_txq_submit_skb(struct fec_enet_priv_tx_q *txq,
fep->hwts_tx_en))
skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
+ if (id_entry->driver_data & FEC_QUIRK_HAS_AVB)
+ estatus |= FEC_TX_BD_FTYPE(queue);
+
if (skb->ip_summed == CHECKSUM_PARTIAL)
estatus |= BD_ENET_TX_PINS | BD_ENET_TX_IINS;
@@ -599,6 +604,7 @@ fec_enet_txq_put_data_tso(struct fec_enet_priv_tx_q *txq, struct sk_buff *skb,
const struct platform_device_id *id_entry =
platform_get_device_id(fep->pdev);
struct bufdesc_ex *ebdp = container_of(bdp, struct bufdesc_ex, desc);
+ unsigned short queue = skb_get_queue_mapping(skb);
unsigned short status;
unsigned int estatus = 0;
dma_addr_t addr;
@@ -629,6 +635,8 @@ fec_enet_txq_put_data_tso(struct fec_enet_priv_tx_q *txq, struct sk_buff *skb,
bdp->cbd_bufaddr = addr;
if (fep->bufdesc_ex) {
+ if (id_entry->driver_data & FEC_QUIRK_HAS_AVB)
+ estatus |= FEC_TX_BD_FTYPE(queue);
if (skb->ip_summed == CHECKSUM_PARTIAL)
estatus |= BD_ENET_TX_PINS | BD_ENET_TX_IINS;
ebdp->cbd_bdu = 0;
@@ -659,6 +667,7 @@ fec_enet_txq_put_hdr_tso(struct fec_enet_priv_tx_q *txq,
platform_get_device_id(fep->pdev);
int hdr_len = skb_transport_offset(skb) + tcp_hdrlen(skb);
struct bufdesc_ex *ebdp = container_of(bdp, struct bufdesc_ex, desc);
+ unsigned short queue = skb_get_queue_mapping(skb);
void *bufaddr;
unsigned long dmabuf;
unsigned short status;
@@ -692,6 +701,8 @@ fec_enet_txq_put_hdr_tso(struct fec_enet_priv_tx_q *txq,
bdp->cbd_datlen = hdr_len;
if (fep->bufdesc_ex) {
+ if (id_entry->driver_data & FEC_QUIRK_HAS_AVB)
+ estatus |= FEC_TX_BD_FTYPE(queue);
if (skb->ip_summed == CHECKSUM_PARTIAL)
estatus |= BD_ENET_TX_PINS | BD_ENET_TX_IINS;
ebdp->cbd_bdu = 0;
--
1.7.8
^ permalink raw reply related
* [PATCH v1 3/4] net: fec: align rx data buffer size for dma map/unmap
From: Fugang Duan @ 2014-09-23 7:40 UTC (permalink / raw)
To: davem; +Cc: netdev, b20596, b38611
In-Reply-To: <1411458058-18934-1-git-send-email-b38611@freescale.com>
Align allocated rx data buffer size for dma map/unmap, otherwise
kernel print warning when enable DMA_API_DEBUG.
Signed-off-by: Fugang Duan <B38611@freescale.com>
---
drivers/net/ethernet/freescale/fec_main.c | 8 +++++---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index e87a8e9..3ce3567 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -1403,7 +1403,8 @@ fec_enet_rx_queue(struct net_device *ndev, int budget, u16 queue_id)
index = fec_enet_get_bd_index(rxq->rx_bd_base, bdp, fep);
data = rxq->rx_skbuff[index]->data;
dma_sync_single_for_cpu(&fep->pdev->dev, bdp->cbd_bufaddr,
- FEC_ENET_RX_FRSIZE, DMA_FROM_DEVICE);
+ FEC_ENET_RX_FRSIZE - fep->rx_align,
+ DMA_FROM_DEVICE);
if (id_entry->driver_data & FEC_QUIRK_SWAP_FRAME)
swap_buffer(data, pkt_len);
@@ -1475,7 +1476,8 @@ fec_enet_rx_queue(struct net_device *ndev, int budget, u16 queue_id)
}
dma_sync_single_for_device(&fep->pdev->dev, bdp->cbd_bufaddr,
- FEC_ENET_RX_FRSIZE, DMA_FROM_DEVICE);
+ FEC_ENET_RX_FRSIZE - fep->rx_align,
+ DMA_FROM_DEVICE);
rx_processing_done:
/* Clear the status flags for this buffer */
status &= ~BD_ENET_RX_STATS;
@@ -2448,7 +2450,7 @@ static void fec_enet_free_buffers(struct net_device *ndev)
if (skb) {
dma_unmap_single(&fep->pdev->dev,
bdp->cbd_bufaddr,
- FEC_ENET_RX_FRSIZE,
+ FEC_ENET_RX_FRSIZE - fep->rx_align,
DMA_FROM_DEVICE);
dev_kfree_skb(skb);
}
--
1.7.8
^ permalink raw reply related
* [PATCH v1 2/4] net: fec: remove the ERR006358 workaround for imx6sx enet
From: Fugang Duan @ 2014-09-23 7:40 UTC (permalink / raw)
To: davem; +Cc: netdev, b20596, b38611
In-Reply-To: <1411458058-18934-1-git-send-email-b38611@freescale.com>
Remove the ERR006358 workaround for imx6sx enet since the hw issue
was fixed on the SOC.
Signed-off-by: Fugang Duan <B38611@freescale.com>
---
drivers/net/ethernet/freescale/fec_main.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index a0f21ce..e87a8e9 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -144,8 +144,8 @@ static struct platform_device_id fec_devtype[] = {
.name = "imx6sx-fec",
.driver_data = FEC_QUIRK_ENET_MAC | FEC_QUIRK_HAS_GBIT |
FEC_QUIRK_HAS_BUFDESC_EX | FEC_QUIRK_HAS_CSUM |
- FEC_QUIRK_HAS_VLAN | FEC_QUIRK_ERR006358 |
- FEC_QUIRK_HAS_AVB | FEC_QUIRK_ERR007885,
+ FEC_QUIRK_HAS_VLAN | FEC_QUIRK_HAS_AVB |
+ FEC_QUIRK_ERR007885,
}, {
/* sentinel */
}
--
1.7.8
^ permalink raw reply related
* [PATCH v1 0/4] net: fec: Code cleanup
From: Fugang Duan @ 2014-09-23 7:40 UTC (permalink / raw)
To: davem; +Cc: netdev, b20596, b38611
This patches does several things:
- Fixing multiqueue issue.
- Removing the unnecessary errata workaround.
- Aligning the data buffer dma map/unmap size.
- Freeing resource after probe failed.
Fugang Duan (4):
net: fec: Add Ftype to BD to distiguish three tx queues for AVB
net: fec: remove the ERR006358 workaround for imx6sx enet
net: fec: align rx data buffer size for dma map/unmap
net: fec: free resource after phy probe failed
drivers/net/ethernet/freescale/fec.h | 1 +
drivers/net/ethernet/freescale/fec_main.c | 25 ++++++++++++++++++++-----
2 files changed, 21 insertions(+), 5 deletions(-)
--
1.7.8
^ permalink raw reply
* Re: [PATCH] macvtap: Fix race between device delete and open.
From: Michael S. Tsirkin @ 2014-09-23 8:31 UTC (permalink / raw)
To: Vladislav Yasevich; +Cc: netdev, Vladislav Yasevich, Jason Wang
In-Reply-To: <1411418057-18937-1-git-send-email-vyasevic@redhat.com>
On Mon, Sep 22, 2014 at 04:34:17PM -0400, Vladislav Yasevich wrote:
> In macvtap device delete and open calls can race and
> this causes a list curruption of the vlan queue_list.
>
> The race intself is triggered by the idr accessors
> that located the vlan device. The device is stored
> into and removed from the idr under both an rtnl and
> a mutex. However, when attempting to locate the device
> in idr, only a mutex is taken. As a result, once cpu
> perfoming a delete may take an rtnl and wait for the mutex,
> while another cput doing an open() will take the idr
> mutex first to fetch the device pointer and later take
> an rtnl to add a queue for the device which may have
> just gotten deleted.
>
> With this patch, we now hold the rtnl for the duration
> of the macvtap_open() call thus making sure that
> open will not race with delete.
>
> CC: Michael S. Tsirkin <mst@redhat.com>
> CC: Jason Wang <jasowang@redhat.com>
> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
Cc: stable@vger.kernel.org
Acked-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> drivers/net/macvtap.c | 18 ++++++++----------
> 1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
> index 3381c4f..0c6adaa 100644
> --- a/drivers/net/macvtap.c
> +++ b/drivers/net/macvtap.c
> @@ -112,17 +112,15 @@ out:
> return err;
> }
>
> +/* Requires RTNL */
> static int macvtap_set_queue(struct net_device *dev, struct file *file,
> struct macvtap_queue *q)
> {
> struct macvlan_dev *vlan = netdev_priv(dev);
> - int err = -EBUSY;
>
> - rtnl_lock();
> if (vlan->numqueues == MAX_MACVTAP_QUEUES)
> - goto out;
> + return -EBUSY;
>
> - err = 0;
> rcu_assign_pointer(q->vlan, vlan);
> rcu_assign_pointer(vlan->taps[vlan->numvtaps], q);
> sock_hold(&q->sk);
> @@ -136,9 +134,7 @@ static int macvtap_set_queue(struct net_device *dev, struct file *file,
> vlan->numvtaps++;
> vlan->numqueues++;
>
> -out:
> - rtnl_unlock();
> - return err;
> + return 0;
> }
>
> static int macvtap_disable_queue(struct macvtap_queue *q)
> @@ -454,11 +450,12 @@ static void macvtap_sock_destruct(struct sock *sk)
> static int macvtap_open(struct inode *inode, struct file *file)
> {
> struct net *net = current->nsproxy->net_ns;
> - struct net_device *dev = dev_get_by_macvtap_minor(iminor(inode));
> + struct net_device *dev;
> struct macvtap_queue *q;
> - int err;
> + int err = -ENODEV;
>
> - err = -ENODEV;
> + rtnl_lock();
> + dev = dev_get_by_macvtap_minor(iminor(inode));
> if (!dev)
> goto out;
>
> @@ -498,6 +495,7 @@ out:
> if (dev)
> dev_put(dev);
>
> + rtnl_unlock();
> return err;
> }
>
> --
> 1.9.3
^ permalink raw reply
* Re: [PATCH net-next v4 2/2] bonding: Simplify the xmit function for modes that use xmit_hash
From: Nikolay Aleksandrov @ 2014-09-23 8:29 UTC (permalink / raw)
To: Mahesh Bandewar
Cc: Jay Vosburgh, Veaceslav Falico, Andy Gospodarek, David Miller,
netdev, Eric Dumazet, Maciej Zenczykowski
In-Reply-To: <CAF2d9jhaRoxXob2mV1wxB=rk5VzGoqyCuBQEmxOSDgfQSxAXBQ@mail.gmail.com>
On 23/09/14 07:13, Mahesh Bandewar wrote:
> On Sun, Sep 21, 2014 at 4:07 AM, Nikolay Aleksandrov <nikolay@redhat.com> wrote:
>> On 09/20/2014 10:04 PM, Mahesh Bandewar wrote:
>>> On Sat, Sep 20, 2014 at 3:19 AM, Nikolay Aleksandrov <nikolay@redhat.com> wrote:
>>>> On 09/20/2014 02:09 AM, Mahesh Bandewar wrote:
>>>>> On Fri, Sep 19, 2014 at 4:06 AM, Nikolay Aleksandrov <nikolay@redhat.com> wrote:
>>>>>>
>>>>>> On 09/19/2014 12:00 PM, Nikolay Aleksandrov wrote:
>>>>>>> On 09/18/2014 11:53 PM, Mahesh Bandewar wrote:
>>>>>>>> Earlier change to use usable slave array for TLB mode had an additional
>>>>>>>> performance advantage. So extending the same logic to all other modes
>>>>>>>> that use xmit-hash for slave selection (viz 802.3AD, and XOR modes).
>>>>>>>> Also consolidating this with the earlier TLB change.
>>>>>>>>
>>>>>>>> The main idea is to build the usable slaves array in the control path
>>>>>>>> and use that array for slave selection during xmit operation.
>>>>>>>>
>>>>>>>> Measured performance in a setup with a bond of 4x1G NICs with 200
>>>>>>>> instances of netperf for the modes involved (3ad, xor, tlb)
>>>>>>>> cmd: netperf -t TCP_RR -H <TargetHost> -l 60 -s 5
>>>>>>>>
>>>>>>>> Mode TPS-Before TPS-After
>>>>>>>>
>>>>>>>> 802.3ad : 468,694 493,101
>>>>>>>> TLB (lb=0): 392,583 392,965
>>>>>>>> XOR : 475,696 484,517
>>>>>>>>
>>>>>>>> Signed-off-by: Mahesh Bandewar <maheshb@google.com>
>>>>>>>> ---
>>>>>>>> v1:
>>>>>>>> (a) If bond_update_slave_arr() fails to allocate memory, it will overwrite
>>>>>>>> the slave that need to be removed.
>>>>>>>> (b) Freeing of array will assign NULL (to handle bond->down to bond->up
>>>>>>>> transition gracefully.
>>>>>>>> (c) Change from pr_debug() to pr_err() if bond_update_slave_arr() returns
>>>>>>>> failure.
>>>>>>>> (d) XOR: bond_update_slave_arr() will consider mii-mon, arp-mon cases and
>>>>>>>> will populate the array even if these parameters are not used.
>>>>>>>> (e) 3AD: Should handle the ad_agg_selection_logic correctly.
>>>>>>>> v2:
>>>>>>>> (a) Removed rcu_read_{un}lock() calls from array manipulation code.
>>>>>>>> (b) Slave link-events now refresh array for all these modes.
>>>>>>>> (c) Moved free-array call from bond_close() to bond_uninit().
>>>>>>>> v3:
>>>>>>>> (a) Fixed null pointer dereference.
>>>>>>>> (b) Removed bond->lock lockdep dependency.
>>>>>>>> v4:
>>>>>>>> (a) Made to changes to comply with Nikolay's locking changes
>>>>>>>> (b) Added a work-queue to refresh slave-array when RTNL is not held
>>>>>>>> (c) Array refresh happens ONLY with RTNL now.
>>>>>>>> (d) alloc changed from GFP_ATOMIC to GFP_KERNEL
>>>>>>>>
>>>>>> <<<snip>>>
>>>>>>>> @@ -3839,6 +4003,7 @@ static void bond_uninit(struct net_device *bond_dev)
>>>>>>>> struct bonding *bond = netdev_priv(bond_dev);
>>>>>>>> struct list_head *iter;
>>>>>>>> struct slave *slave;
>>>>>>>> + struct bond_up_slave *arr;
>>>>>>>>
>>>>>>>> bond_netpoll_cleanup(bond_dev);
>>>>>>>>
>>>>>>>> @@ -3847,6 +4012,12 @@ static void bond_uninit(struct net_device *bond_dev)
>>>>>>>> __bond_release_one(bond_dev, slave->dev, true);
>>>>>>>> netdev_info(bond_dev, "Released all slaves\n");
>>>>>>>>
>>>>>> Sorry but I just spotted a major problem, bond_3ad_unbind_slave() (called
>>>>>> from __bond_release_one) calls ad_agg_selection_logic() which can re-arm
>>>>>> the slave_arr work after it's supposed to be stopped here (i.e. the bond
>>>>>> device has been closed so all works should've been stopped) so we might
>>>>>> leak memory and access freed memory after all since it'll keep
>>>>>> re-scheduling itself until it can acquire rtnl which is after the bond
>>>>>> device has been destroyed.
>>>>>>
>>>>> This should not be a problem. ndo_close (bond_close()) is called
>>>>> before ndo_uninit(bond_uninit()), so the work-queues get cancelled
>>>>> there so if rearm tries to schedule some work after queue gets
>>>>> cancelled, it can't do much and wont harm anything.
>>>>> Hence there wont be any arrays built once it's free-ed completely and
>>>>> therefore no memory leak. I addded some instrumentation and tried
>>>>> following sequence -
>>>>>
>>>>> # modprobe bonding mode=4
>>>>> # ip link set bond0 up
>>>>> # [Add ip]
>>>>> # [Add default route]
>>>>> # ifenslave bond0 eth0 eth1 eth2 eth3
>>>>> ....
>>>>> [Run some backgound traffic. I used netperf.]
>>>>>
>>>>> # ip link bond0 down
>>>>>
>>>>> I did not see anything "bad" happening. Did your trial produced
>>>>> something unpleasant?
>>>>>
>>>> The test you've done is irrelevant to the situation that I described
>>>> because ndo_uninit() is called when the device is being destroyed. Moreover
>>>> the case I told you about would require to have an active aggregator and an
>>>> inactive one (i.e. so agg selection logic will get called), here is the result:
>>>> [ 428.916586] bond1 (unregistering): Removing an active aggregator
>>>> [ 428.916589] Failed to build slave-array.
>>>> [ 428.916849] bond1 (unregistering): Releasing active interface eth1
>>>> [ 428.920342] bond1 (unregistering): Released all slaves
>>>> [ 428.923043] Failed to update slave array from WT
>>>> [ 428.924098] Failed to update slave array from WT
>>>> [ 428.925125] Failed to update slave array from WT
>>>> [ 428.926120] Failed to update slave array from WT
>>>> [ 428.927096] Failed to update slave array from WT
>>>> [ 428.928101] Failed to update slave array from WT
>>>> [ 428.929120] Failed to update slave array from WT
>>>> [ 428.930086] BUG: unable to handle kernel NULL pointer dereference at
>>>> (null)
>>>> [ 428.930644] IP: [<ffffffff810aa37b>] __queue_work+0x7b/0x350
>>>> [ 428.930946] PGD 0
>>>> [ 428.931053] Oops: 0000 [#1] SMP
>>>> [ 428.931053] Modules linked in: sfc ptp pps_core mdio i2c_algo_bit mtd
>>>> bonding(O) snd_hda_codec_generic joydev crct10dif_pclmul crc32_pclmul
>>>> i2c_piix4 ppdev crc32c_intel ghash_clmulni_intel parport_pc snd_hda_intel
>>>> snd_hda_controller snd_hda_codec snd_hwdep snd_pcm snd_timer 9pnet_virtio
>>>> snd 9pnet pcspkr parport i2ccore serio_raw virtio_console virtio_balloon
>>>> pvpanic soundcore virtio_blk virtio_net ata_generic floppy pata_acpi
>>>> virtio_pci virtio_ring virtio
>>>> [ 428.935022] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G O
>>>> 3.17.0-rc4+ #30
>>>> [ 428.935022] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
>>>> [ 428.935022] task: ffffffff81c1b460 ti: ffffffff81c00000 task.ti:
>>>> ffffffff81c00000
>>>> [ 428.935022] RIP: 0010:[<ffffffff810aa37b>] [<ffffffff810aa37b>]
>>>> __queue_work+0x7b/0x350
>>>> [ 428.935022] RSP: 0018:ffff88005f003e28 EFLAGS: 00010086
>>>> [ 428.935022] RAX: ffff88005c05c800 RBX: 0000000000000000 RCX:
>>>> 0000000000000000
>>>> [ 428.935022] RDX: 0000000000000000 RSI: 0000000000000006 RDI:
>>>> ffff88005a4fbd58
>>>> [ 428.935022] RBP: ffff88005f003e60 R08: 0000000000000046 R09:
>>>> ffffffff8225abc2
>>>> [ 428.935022] R10: 0000000000000004 R11: 0000000000000005 R12:
>>>> ffff88005a4fbd58
>>>> [ 428.935022] R13: 0000000000000008 R14: ffff88004b211800 R15:
>>>> 00000000000102f0
>>>> [ 428.935022] FS: 0000000000000000(0000) GS:ffff88005f000000(0000)
>>>> knlGS:0000000000000000
>>>> [ 428.935022] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>>> [ 428.935022] CR2: 0000000000000000 CR3: 000000004abde000 CR4:
>>>> 00000000000406f0
>>>> [ 428.935022] Stack:
>>>> [ 428.935022] 0a19522f72b12222 0000000081c1b460 ffffffff8225abc0
>>>> ffff88005a4fbd78
>>>> [ 428.935022] 0000000000000101 ffffffff810aa650 ffff88005a4fbd58
>>>> ffff88005f003e70
>>>> [ 428.935022] ffffffff810aa668 ffff88005f003ea8 ffffffff810f3536
>>>> ffffffff8225abc0
>>>> [ 428.935022] Call Trace:
>>>> [ 428.935022] <IRQ>
>>>> [ 428.935022]
>>>> [ 428.935022] [<ffffffff810aa650>] ? __queue_work+0x350/0x350
>>>> [ 428.935022] [<ffffffff810aa668>] delayed_work_timer_fn+0x18/0x20
>>>> [ 428.935022] [<ffffffff810f3536>] call_timer_fn+0x36/0x120
>>>> [ 428.935022] [<ffffffff810aa650>] ? __queue_work+0x350/0x350
>>>> [ 428.935022] [<ffffffff810f38f5>] run_timer_softirq+0x1a5/0x320
>>>> [ 428.935022] [<ffffffff81096dc5>] __do_softirq+0xf5/0x2b0
>>>> [ 428.935022] [<ffffffff810971fd>] irq_exit+0xbd/0xd0
>>>> [ 428.935022] [<ffffffff8173b715>] smp_apic_timer_interrupt+0x45/0x60
>>>> [ 428.935022] [<ffffffff8173981d>] apic_timer_interrupt+0x6d/0x80
>>>> [ 428.935022] <EOI>
>>>> [ 428.935022]
>>>> [ 428.935022] [<ffffffff810581c6>] ? native_safe_halt+0x6/0x10
>>>> [ 428.935022] [<ffffffff8101f36f>] default_idle+0x1f/0xe0
>>>> [ 428.935022] [<ffffffff8101fd8f>] arch_cpu_idle+0xf/0x20
>>>> [ 428.935022] [<ffffffff810d25dd>] cpu_startup_entry+0x38d/0x3c0
>>>> [ 428.935022] [<ffffffff81722927>] rest_init+0x87/0x90
>>>> [ 428.935022] [<ffffffff81d3510e>] start_kernel+0x482/0x4a3
>>>> [ 428.935022] [<ffffffff81d34a85>] ? set_init_arg+0x53/0x53
>>>> [ 428.935022] [<ffffffff81d34120>] ? early_idt_handlers+0x120/0x120
>>>> [ 428.935022] [<ffffffff81d345ee>] x86_64_start_reservations+0x2a/0x2c
>>>> [ 428.935022] [<ffffffff81d3473d>] x86_64_start_kernel+0x14d/0x170
>>>> [ 428.935022] Code: 84 bb 01 00 00 a8 02 0f 85 eb 00 00 00 48 63 45 d4 49
>>>> 8b 9e 08 01 00 00 48 03 1c c5 60 fa d0 81 4c 89 e7 e8 18 f5 ff ff 48 85 c0
>>>> <48> 8b 3b 0f 84 7c 01 00 00 48 39 c7 0f 84 73 01 00 00 48 89 c7
>>>> [ 428.935022] RIP [<ffffffff810aa37b>] __queue_work+0x7b/0x350
>>>> [ 428.935022] RSP <ffff88005f003e28>
>>>> [ 428.935022] CR2: 0000000000000000
>>>>
>>>> This is because it keeps trying to re-schedule even though the interface's
>>>> memory has been freed.
>>>>
>>> Hmm, how do we handle this?
>>>
>> This is tricky and what concerns me more is that people might make this
>> mistake again in the future. It's easy to unknowingly make use of a
>> function that re-schedules this from the wrong place.
>> What I just noticed is that for all 3ad cases you could pull the scheduling
>> in the bond_3ad_state_machine_handler() function.
>> The call sites of ad_agg_selection_logic() are:
>> - 3ad unbind slave (no need to schedule here as __bond_release_one would
>> rebuild the array anyhow)
>> - bond_3ad_state_machine_handler() <- here's where the schedule should
>> happen as this gets stopped first when the bond is closed and can't get
>> restarted unless it's opened again.
>> - ad_port_selection_logic() <- this is called from
>> bond_3ad_state_machine_handler() only, so this case will be handled as well.
>>
>> The other 2 functions that you convert - ad_enable/disable_collecting are
>> used only from ad_mux_machine() which is only called in
>> bond_3ad_state_machine_handler().
>>
>> So basically you can pull all rebuild schedules in their common caller -
>> bond_3ad_state_machine_handler(), just make a flag to note that a rebuild
>> is needed probably something similar to should_notify_rtnl.
>> This way you can remove the scheduling from the various 3ad functions that
>> may get used and will have it only in 1 place which is more easily controlled.
>>
> Well, I was just trying to avoid using flags to pass state from one to
> another function so that we can update the array at one place. This
> might introduce some bug so I was keeping it simple and build it only
> when the condition requires it to build it. However I do not see how
> this will fix the issue that you have seen, or would it? If so how?
>
You don't have to pass state between functions, you just have to collect the
return values from them in the single caller and see if scheduling an update is
required in the end. Obviously I haven't tested this fix, but the reasoning
behind it goes like this:
The usual device destruction goes like: 1. ndo_close() 2. ndo_uninit()... So
when bond_close() is executed the 3ad workqueue will get stopped first, and then
the slave_update workqueue will get stopped (note - the order is important,
since the only place where the slave_update workqueue gets scheduled to run is
from the 3ad workqueue function). So when we reach bond_uninit() there's no way
for the 3ad workqueue function to run and we're 100% sure that the slave_update
workqueue has been canceled as well.
The reason for this is because the 3ad workqueue function is started in
bond_open() which obviously can't run without rtnl held, which is why the other
workqueue functions also are stopped in the same manner.
So basically, the idea is that you have only 1 place from which you can schedule
the slave_update array and we can guarantee that it cannot get called once the
bond device has been closed (bond_close()). You must not do a slave array update
schedule in bond_3ad_unbind_slave, but that is okay because the slave array will
get updated by __bond_release_one() anyhow.
>> Of course, the alternative would be once again - convert
>> bond_3ad_state_machine_handler() to RTNL, but that has its own set of problems.
>>
> It's convoluted, let's keep it simple for now :)
>
>>>> While testing this I spotted another issue as well - Failed to build
>>>> slave_arr message has been printed too many times because you print it in
>>>> 3ad mode when there's no active aggregator (bond_3ad_get_active_agg_info
>>>> check in bond_update_slave_arr) which leads to re-scheduling which also
>>>> lead to a deadlock.
>>>>
>>> I think this can be corrected with pr_ratelimited() call.
>>>
>> IMO it shouldn't print anything if it couldn't rebuild the array due to
>> missing active aggregator as that's not an error condition. It should
>> though probably clean out the slave array because transmission shouldn't be
>> possible without an active aggregator in 3ad.
>>
> Sure missing active aggregator is not an error but free-ing the slave
> array silently would be bad either. At least we would see something in
> the messages about "something" went wrong.
Nothing has went wrong, not having an active aggregator is a normal state that
can happen and in fact could be the state while the bond device is configured.
It is not advisable to spit out errors in such case as there has been no error
condition to begin with. Dealing with failed active aggregator and notifying the
user of it and so on is the job of the 3ad code, not of the slave_update mechanism.
One more thing you really should make sure that we don't xmit when there's no
active aggregator, it doesn't make sense otherwise and it is actually the
current behaviour (check bond_3ad_xmit_xor(), first thing it does is try to
obtain active aggregator and if it fails - it drops the packet, the error
condition there has been marked as netdev_dbg() so it can be enabled only per
request of the user and isn't printed normally).
Moreover it's really a bad idea to reschedule the slave array rebuilding if
there's no active aggregator because it may be the case we don't have it for a
long time and it will cause constant rtnl acquire/release cycles.
>>
>>
>>
^ permalink raw reply
* [PATCH net] r8152: fix the carrier off when autoresuming
From: Hayes Wang @ 2014-09-23 8:31 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA
Cc: nic_swsd-Rasf1IRRPZFBDgjK7y7TUQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA, Hayes Wang
netif_carrier_off would be called when autoresuming, even though
the cable is plugged. This causes some applications do relative
actions when detecting the carrier off. Keep the status of the
carrier, and let it be modified when the linking change occurs.
Signed-off-by: Hayes Wang <hayeswang-Rasf1IRRPZFBDgjK7y7TUQ@public.gmane.org>
---
drivers/net/usb/r8152.c | 52 +++++++++++++++++++++++++++++++++----------------
1 file changed, 35 insertions(+), 17 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 74760e8..e039442 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -1949,10 +1949,34 @@ static void rxdy_gated_en(struct r8152 *tp, bool enable)
ocp_write_word(tp, MCU_TYPE_PLA, PLA_MISC_1, ocp_data);
}
+static int rtl_start_rx(struct r8152 *tp)
+{
+ int i, ret = 0;
+
+ INIT_LIST_HEAD(&tp->rx_done);
+ for (i = 0; i < RTL8152_MAX_RX; i++) {
+ INIT_LIST_HEAD(&tp->rx_info[i].list);
+ ret = r8152_submit_rx(tp, &tp->rx_info[i], GFP_KERNEL);
+ if (ret)
+ break;
+ }
+
+ return ret;
+}
+
+static int rtl_stop_rx(struct r8152 *tp)
+{
+ int i;
+
+ for (i = 0; i < RTL8152_MAX_RX; i++)
+ usb_kill_urb(tp->rx_info[i].urb);
+
+ return 0;
+}
+
static int rtl_enable(struct r8152 *tp)
{
u32 ocp_data;
- int i, ret;
r8152b_reset_packet_filter(tp);
@@ -1962,14 +1986,7 @@ static int rtl_enable(struct r8152 *tp)
rxdy_gated_en(tp, false);
- INIT_LIST_HEAD(&tp->rx_done);
- ret = 0;
- for (i = 0; i < RTL8152_MAX_RX; i++) {
- INIT_LIST_HEAD(&tp->rx_info[i].list);
- ret |= r8152_submit_rx(tp, &tp->rx_info[i], GFP_KERNEL);
- }
-
- return ret;
+ return rtl_start_rx(tp);
}
static int rtl8152_enable(struct r8152 *tp)
@@ -2053,8 +2070,7 @@ static void rtl_disable(struct r8152 *tp)
mdelay(1);
}
- for (i = 0; i < RTL8152_MAX_RX; i++)
- usb_kill_urb(tp->rx_info[i].urb);
+ rtl_stop_rx(tp);
rtl8152_nic_reset(tp);
}
@@ -3083,13 +3099,14 @@ static int rtl8152_suspend(struct usb_interface *intf, pm_message_t message)
clear_bit(WORK_ENABLE, &tp->flags);
usb_kill_urb(tp->intr_urb);
cancel_delayed_work_sync(&tp->schedule);
+ tasklet_disable(&tp->tl);
if (test_bit(SELECTIVE_SUSPEND, &tp->flags)) {
+ rtl_stop_rx(tp);
rtl_runtime_suspend_enable(tp, true);
} else {
- tasklet_disable(&tp->tl);
tp->rtl_ops.down(tp);
- tasklet_enable(&tp->tl);
}
+ tasklet_enable(&tp->tl);
}
return 0;
@@ -3108,17 +3125,18 @@ static int rtl8152_resume(struct usb_interface *intf)
if (test_bit(SELECTIVE_SUSPEND, &tp->flags)) {
rtl_runtime_suspend_enable(tp, false);
clear_bit(SELECTIVE_SUSPEND, &tp->flags);
+ set_bit(WORK_ENABLE, &tp->flags);
if (tp->speed & LINK_STATUS)
- tp->rtl_ops.disable(tp);
+ rtl_start_rx(tp);
} else {
tp->rtl_ops.up(tp);
rtl8152_set_speed(tp, AUTONEG_ENABLE,
tp->mii.supports_gmii ? SPEED_1000 : SPEED_100,
DUPLEX_FULL);
+ tp->speed = 0;
+ netif_carrier_off(tp->netdev);
+ set_bit(WORK_ENABLE, &tp->flags);
}
- tp->speed = 0;
- netif_carrier_off(tp->netdev);
- set_bit(WORK_ENABLE, &tp->flags);
usb_submit_urb(tp->intr_urb, GFP_KERNEL);
}
--
1.9.3
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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
* Re: [PATCHv2] ipv4: Do not cache routing failures due to disabled forwarding.
From: Nicolas Cavallari @ 2014-09-23 8:34 UTC (permalink / raw)
To: David Miller; +Cc: netdev, kuznet, jmorris, yoshfuji, kaber
In-Reply-To: <541894F6.3090903@green-communications.fr>
On 16/09/2014 21:52, Nicolas Cavallari wrote:
> On 16/09/2014 20:54, David Miller wrote:
>> From: Nicolas Cavallari <nicolas.cavallari@green-communications.fr>
>> Date: Mon, 15 Sep 2014 12:28:13 +0200
>>
>>> If we cache them, the kernel will reuse them, independently of
>>> whether forwarding is enabled or not. Which means that if forwarding is
>>> disabled on the input interface where the first routing request comes
>>> from, then that unreachable result will be cached and reused for
>>> other interfaces, even if forwarding is enabled on them.
>>>
>>> This can be verified with two interfaces A and B and an output interface
>>> C, where B has forwarding enabled, but not A and trying
>>> ip route get $dst iif A from $src && ip route get $dst iif B from $src
>>>
>>> Signed-off-by: Nicolas Cavallari <nicolas.cavallari@green-communications.fr>
>>> ---
>>> v2: simplify patch using julian anastasov's suggestion.
>>
>> This also disables caching for the cases of a simple fib lookup failure.
>
> Caching is already not done on a fib lookup failure. On which fib_info
> would you cache anyway ? res.fi is already NULL in this case.
>
>> Handle cached route invalidation the way it's meant to be, by bumping
>> the rt_genid.
>>
>> diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
>> index 214882e..aa4e63c 100644
>> --- a/net/ipv4/devinet.c
>> +++ b/net/ipv4/devinet.c
>> @@ -1965,6 +1965,8 @@ static void inet_forward_change(struct net *net)
>> }
>> rcu_read_unlock();
>> }
>> +
>> + rt_cache_flush(net);
>> }
>>
>> static int devinet_conf_ifindex(struct net *net, struct ipv4_devconf *cnf)
>
> This doesn't solve the problem. Flushing the cache only changes how the
> bug manifests:
>
> ip route flush cache
> ip route get 10.0.0.1 from 10.0.0.2 iif A
> # unreachable (because forwarding on A is disabled). This is cached.
> ip route get 10.0.0.1 from 10.0.0.2 iif B
> # unreachable (at first fib_lookup finds it reachable and forwarding on
> B is enabled. And then there is a valid cache entry, which is reused
> blindly even if it says "unreachable").
>
> ip route flush cache
> ip route get 10.0.0.1 from 10.0.0.2 iif B
> # reachable (because forwarding on B is enabled). This is cached
> ip route get 10.0.0.1 from 10.0.0.2 iif A
> # reachable (forwarding on A is disabled, so it is unreachable
> initially. But there is a valid cache entry, which says "reachable",
> which is reused)
Any news on this patch ? Is there anything I can do so that this bug
is finally fixed ?
^ permalink raw reply
* [net-next] vxlan: Fix bug introduced by commit acbf74a76300
From: Andy Zhou @ 2014-09-23 8:44 UTC (permalink / raw)
To: davem; +Cc: netdev, Andy Zhou, Eric Dumazet
Commit acbf74a76300 ("vxlan: Refactor vxlan driver to make use of the common UDP tunnel functions." introduced a bug in vxlan_xmit_one()
function, causing it to transmit Vxlan packets without proper
Vxlan header inserted. The change was not needed in the first
place. Revert it.
Reported-by: Tom Herbert <therbert@google.com>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Andy Zhou <azhou@nicira.com>
---
drivers/net/vxlan.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 39c8653..34e102e 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -1778,11 +1778,11 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
tos = ip_tunnel_ecn_encap(tos, old_iph, skb);
ttl = ttl ? : ip4_dst_hoplimit(&rt->dst);
- err = udp_tunnel_xmit_skb(vxlan->vn_sock->sock, rt, skb,
- fl4.saddr, dst->sin.sin_addr.s_addr,
- tos, ttl, df, src_port, dst_port,
- !net_eq(vxlan->net,
- dev_net(vxlan->dev)));
+ err = vxlan_xmit_skb(vxlan->vn_sock, rt, skb,
+ fl4.saddr, dst->sin.sin_addr.s_addr,
+ tos, ttl, df, src_port, dst_port,
+ htonl(vni << 8),
+ !net_eq(vxlan->net, dev_net(vxlan->dev)));
if (err < 0)
goto rt_tx_error;
--
1.7.9.5
^ permalink raw reply related
* [PATCH] net: optimise csum_replace4()
From: Christophe Leroy @ 2014-09-23 8:54 UTC (permalink / raw)
To: David S. Miller; +Cc: linux-kernel, Eric Dumazet, netdev
csum_partial() is a generic function which is not optimised for small fixed
length calculations, and its use requires to store "from" and "to" values in
memory while we already have them available in registers. This also has impact,
especially on RISC processors. In the same spirit as the change done by
Eric Dumazet on csum_replace2(), this patch rewrites inet_proto_csum_replace4()
taking into account RFC1624.
I spotted during a NATted tcp transfert that csum_partial() is one of top 5
consuming functions (around 8%), and the second user of csum_partial() is
inet_proto_csum_replace4().
I have proposed the same modification to inet_proto_csum_replace4() in another
patch.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
include/net/checksum.h | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/include/net/checksum.h b/include/net/checksum.h
index 87cb190..6465bae 100644
--- a/include/net/checksum.h
+++ b/include/net/checksum.h
@@ -122,9 +122,7 @@ static inline __wsum csum_partial_ext(const void *buff, int len, __wsum sum)
static inline void csum_replace4(__sum16 *sum, __be32 from, __be32 to)
{
- __be32 diff[] = { ~from, to };
-
- *sum = csum_fold(csum_partial(diff, sizeof(diff), ~csum_unfold(*sum)));
+ *sum = csum_fold(csum_add(csum_sub(~csum_unfold(*sum), from), to));
}
/* Implements RFC 1624 (Incremental Internet Checksum)
--
2.1.0
^ permalink raw reply related
* [PATCH] net: optimise inet_proto_csum_replace4()
From: Christophe Leroy @ 2014-09-23 8:54 UTC (permalink / raw)
To: David S. Miller; +Cc: linux-kernel, Eric Dumazet, netdev
csum_partial() is a generic function which is not optimised for small fixed
length calculations, and its use requires to store "from" and "to" values in
memory while we already have them available in registers. This also has impact,
especially on RISC processors. In the same spirit as the change done by
Eric Dumazet on csum_replace2(), this patch rewrites inet_proto_csum_replace4()
taking into account RFC1624.
I spotted during a NATted tcp transfert that csum_partial() is one of top 5
consuming functions (around 8%), and the second user of csum_partial() is
inet_proto_csum_replace4().
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
--
net/core/utils.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/net/core/utils.c b/net/core/utils.c
index eed3433..efc76dd 100644
--- a/net/core/utils.c
+++ b/net/core/utils.c
@@ -306,16 +306,14 @@ EXPORT_SYMBOL(in6_pton);
void inet_proto_csum_replace4(__sum16 *sum, struct sk_buff *skb,
__be32 from, __be32 to, int pseudohdr)
{
- __be32 diff[] = { ~from, to };
if (skb->ip_summed != CHECKSUM_PARTIAL) {
- *sum = csum_fold(csum_partial(diff, sizeof(diff),
- ~csum_unfold(*sum)));
+ *sum = csum_fold(csum_add(csum_sub(~csum_unfold(*sum), from),
+ to));
if (skb->ip_summed == CHECKSUM_COMPLETE && pseudohdr)
- skb->csum = ~csum_partial(diff, sizeof(diff),
- ~skb->csum);
+ skb->csum = ~csum_add(csum_sub(~(skb->csum), from), to);
} else if (pseudohdr)
- *sum = ~csum_fold(csum_partial(diff, sizeof(diff),
- csum_unfold(*sum)));
+ *sum = ~csum_fold(csum_add(csum_sub(csum_unfold(*sum), from),
+ to));
}
EXPORT_SYMBOL(inet_proto_csum_replace4);
--
2.1.0
^ permalink raw reply related
* [PATCHv4 1/1] bluetooth: Check for SCO type before setting retransmission effort
From: Bernhard Thaler @ 2014-09-23 9:01 UTC (permalink / raw)
To: marcel, gustavo, johan.hedberg, davem
Cc: linux-bluetooth, netdev, linux-kernel, Bernhard Thaler
In-Reply-To: <1411035321-10215-1-git-send-email-bernhard.thaler@r-it.at>
SCO connection cannot be setup to devices that do not support retransmission.
Patch based on http://permalink.gmane.org/gmane.linux.bluez.kernel/7779 and
adapted for this kernel version.
Code changed to check SCO/eSCO type before setting retransmission effort
and max. latency. The purpose of the patch is to support older devices not
capable of eSCO.
Tested on Blackberry 655+ headset which does not support retransmission.
Credits go to Alexander Sommerhuber.
Signed-off-by: Bernhard Thaler <bernhard.thaler@r-it.at>
---
net/bluetooth/hci_conn.c | 22 +++++++++++++++++-----
1 file changed, 17 insertions(+), 5 deletions(-)
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index faff624..afa325b 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -38,7 +38,7 @@ struct sco_param {
u16 max_latency;
};
-static const struct sco_param sco_param_cvsd[] = {
+static const struct sco_param esco_param_cvsd[] = {
{ EDR_ESCO_MASK & ~ESCO_2EV3, 0x000a }, /* S3 */
{ EDR_ESCO_MASK & ~ESCO_2EV3, 0x0007 }, /* S2 */
{ EDR_ESCO_MASK | ESCO_EV3, 0x0007 }, /* S1 */
@@ -46,6 +46,11 @@ static const struct sco_param sco_param_cvsd[] = {
{ EDR_ESCO_MASK | ESCO_HV1, 0xffff }, /* D0 */
};
+static const struct sco_param sco_param_cvsd[] = {
+ { EDR_ESCO_MASK | ESCO_HV3, 0xffff }, /* D1 */
+ { EDR_ESCO_MASK | ESCO_HV1, 0xffff }, /* D0 */
+};
+
static const struct sco_param sco_param_wideband[] = {
{ EDR_ESCO_MASK & ~ESCO_2EV3, 0x000d }, /* T2 */
{ EDR_ESCO_MASK | ESCO_EV3, 0x0008 }, /* T1 */
@@ -194,10 +199,17 @@ bool hci_setup_sync(struct hci_conn *conn, __u16 handle)
param = &sco_param_wideband[conn->attempt - 1];
break;
case SCO_AIRMODE_CVSD:
- if (conn->attempt > ARRAY_SIZE(sco_param_cvsd))
- return false;
- cp.retrans_effort = 0x01;
- param = &sco_param_cvsd[conn->attempt - 1];
+ if (lmp_esco_capable(conn->link)) {
+ if (conn->attempt > ARRAY_SIZE(esco_param_cvsd))
+ return false;
+ cp.retrans_effort = 0x01;
+ param = &esco_param_cvsd[conn->attempt - 1];
+ } else {
+ if (conn->attempt > ARRAY_SIZE(sco_param_cvsd))
+ return false;
+ cp.retrans_effort = 0xff;
+ param = &sco_param_cvsd[conn->attempt - 1];
+ }
break;
default:
return false;
--
1.7.10.4
^ permalink raw reply related
* Re: 3.17 kernel crash while loading IPoIB
From: Eric Dumazet @ 2014-09-23 9:05 UTC (permalink / raw)
To: Sharma, Karun; +Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <637AD4DF2E595346BAB0DF8C46F3A2B003291F06@BGSMSX101.gar.corp.intel.com>
On Tue, 2014-09-23 at 05:15 +0000, Sharma, Karun wrote:
> Hello:
>
> I am facing an issue wherein kernel 3.17 crashes while loading IPoIB
> module. I guess the issue discussed in this thread
> (https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg20963.html) is similar.
>
> We were able to reproduce the issue with RC6 also. Here are the steps
> I followed:
>
> I compiled and installed 3.17 kernel on top of RHEL 6.5.
> Then I changed rdma.conf to not load IPoIB (If I don't do this, the
> kernel crashes while booting and starting RDMA service.)
> After the server comes up, I just did "modprobe ib_ipoib" and kernel
> crashes.
> Please see below the kernel back trace.
>
> Seeing the announcement, it looks like RC6 will be the last RC for
> 3.17 kernel. Will the release happen with this issue? Is there any
> workaround available for this issue?
> I am not sure what mechanism/process is used to report issue to kernel
> community.
>
> Regards
> Karun
This seems fixed now :
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=257117862634d89de33fec74858b1a0ba5ab444b
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=b49fe36208b45f76dfbcfcd3afd952a33fa9f5ce
^ permalink raw reply
* Re: [PATCH net-next 3/3] tcp: better TCP_SKB_CB layout to reduce cache line misses
From: Eric Dumazet @ 2014-09-23 9:09 UTC (permalink / raw)
To: Christoph Paasch
Cc: Eric Dumazet, David S. Miller, netdev, Yuchung Cheng,
Neal Cardwell
In-Reply-To: <20140923073653.GM3291@cpaasch-mac>
On Tue, 2014-09-23 at 09:36 +0200, Christoph Paasch wrote:
> On 23/09/14 - 09:20:16, Christoph Paasch wrote:
> > Hello Eric,
...
> > doesn't also the tx-path relies on IPCB(skb) being memset to 0 (e.g., in
> > xfrm4_output or ip_options_build)
>
> Crap, forget about ip_options_build. It is memcpy'ing into IPCB... Confused
> src with dest... :-)
>
> Nevertheless, wouldn't there be a problem in xfrm4_output?
Hi Christoph.
I believe you're right. I'll include a memset() in v2.
Thanks !
^ permalink raw reply
* Re: [PATCH v2] brcmfmac: Fix off by one bug in brcmf_count_20mhz_channels()
From: Arend van Spriel @ 2014-09-23 9:10 UTC (permalink / raw)
To: Emil Goode
Cc: Brett Rudley, Franky (Zhenhui) Lin, Hante Meuleman,
John W. Linville, Pieter-Paul Giesberts, Daniel Kim,
linux-wireless, brcm80211-dev-list, netdev, linux-kernel,
kernel-janitors
In-Reply-To: <1411426195-13467-1-git-send-email-emilgoode@gmail.com>
On 09/23/14 00:49, Emil Goode wrote:
> In the brcmf_count_20mhz_channels function we are looping through a list
> of channels received from firmware. Since the index of the first channel
> is 0 the condition leads to an off by one bug. This is causing us to hit
> the WARN_ON_ONCE(1) calls in the brcmu_d11n_decchspec function, which is
> how I discovered the bug.
>
> Introduced by:
> commit b48d891676f756d48b4d0ee131e4a7a5d43ca417
> ("brcmfmac: rework wiphy structure setup")
>
Hi John,
This bug was introduced in 3.17 so can it still go in the wireless tree?
I verified it applies to wireless/master branch.
Regards,
Arend
> Acked-by: Arend van Spriel<arend@broadcom.com>
> Signed-off-by: Emil Goode<emilgoode@gmail.com>
> ---
> v2: Added Arends "Acked-by" tag.
>
> drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
> index 12a60ca..0517687 100644
> --- a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
> +++ b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
> @@ -4924,7 +4924,7 @@ static void brcmf_count_20mhz_channels(struct brcmf_cfg80211_info *cfg,
> struct brcmu_chan ch;
> int i;
>
> - for (i = 0; i<= total; i++) {
> + for (i = 0; i< total; i++) {
> ch.chspec = (u16)le32_to_cpu(chlist->element[i]);
> cfg->d11inf.decchspec(&ch);
>
^ permalink raw reply
* Re: [PATCH net-next v2 2/5] net: tcp: add flag for ca to indicate that ECN is required
From: Florian Westphal @ 2014-09-23 9:11 UTC (permalink / raw)
To: David Miller
Cc: fw, hagen, lars, eric.dumazet, fontana, hannes, glenn.judd,
dborkman, netdev
In-Reply-To: <20140922.163357.553905454314637491.davem@davemloft.net>
Hi Dave,
David Miller <davem@davemloft.net> wrote:
> From: Florian Westphal <fw@strlen.de>
> Date: Sat, 20 Sep 2014 23:29:19 +0200
>
> > From: Daniel Borkmann <dborkman@redhat.com>
> >
> > This patch adds a flag to TCP congestion algorithms that allows
> > for requesting to mark IPv4/IPv6 sockets with transport as ECN
> > capable, that is, ECT(0), when required by a congestion algorithm.
> >
> > It is currently used and needed in DataCenter TCP (DCTCP), as it
> > requires both peers to assert ECT on all IP packets sent - it
> > uses ECN feedback (i.e. CE, Congestion Encountered information)
> > from switches inside the data center to derive feedback to the
> > end hosts.
> >
> > Therefore, simply add a new flag to icsk_ca_ops. Note that DCTCP's
> > algorithm/behaviour slightly diverges from RFC3168, therefore this
> > is only (!) enabled iff the assigned congestion control ops module
> > has requested this. By that, we can tightly couple this logic really
> > only to the provided congestion control ops.
> >
> > Joint work with Florian Westphal and Glenn Judd.
> >
> > Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> > Signed-off-by: Florian Westphal <fw@strlen.de>
> > Signed-off-by: Glenn Judd <glenn.judd@morganstanley.com>
>
> I don't think any administrator is going to be happy with this behavior.
>
> If he explicitly sets the tcp_ecn sysctl to zero, and then an
> unprivileged user can just start emitting ECN bits by selecting a
> different congestion control algorithm, that is unexpected.
>
> Please instead make datacenter TCP require ECN to be enabled.
Thanks for your feedback! We have actually thought about that for
quite a while before starting on the implementation, and concluded
that the behaviour is actually fine as is for three reasons:
1) DCTCP is very tighly coupled to the ECN machinery as described
in the paper. Not having ECN enabled and nevertheless allowing
DCTCP (if we would implement above feedback) would just make it
fallback to Reno, which would not be useful in the first place.
So an administrator would rather not load DCTCP to the available
congestion control modules in the first place in this case.
2) Right now an administrator can choose to use DCTCP only for a
particular process, and still avoid exposing ECN to the outside
world for every other process.
3) An unpriviledged user would not be able to use DCTCP *unless*
an administrator has explicitly allowed to use it. This is being
reflected since Stephen's commit ce7bc3bf15cb ("[TCP]: Restrict
congestion control choices.") where only Reno or the currently
compiled-in default choice is non-restricted.
If you nevertheless think that it is useful to include above feedback
to overcome your objection, we could just add a check for tcp_ecn
sysctl being set to 1 for the initialization of the congestion control
ops of the socket and otherwise fall back to Reno.
Thanks!
Daniel and Florian
^ 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