* Re: [net-next PATCH v1 1/3] net: sched: af_packet support for direct ring access
From: David Miller @ 2014-10-07 17:01 UTC (permalink / raw)
To: alexei.starovoitov
Cc: danny.zhou, willemb, john.fastabend, dborkman, fw, gerlitz.or,
hannes, netdev, john.ronciak, amirv, eric.dumazet
In-Reply-To: <CAADnVQJf8jtRr3xBdSOt_X188Xkkg0EnxVd+gh6ruTMjc9jOHg@mail.gmail.com>
From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Date: Tue, 7 Oct 2014 09:33:04 -0700
> I guess that's great for DPDK users, but I don't think it's good for
> linux.
Any use of a piece of hardware is fine with me, personally, as long
as adequate protections are in place.
If it's just a descriptor ring in software and a doorbell to trigger
a refetch of the head and tail pointers, with appropriate protection
and control of the memory attached to the ring, I don't see how I
could object to such a facility.
^ permalink raw reply
* Re: [PATCH] net: bcmgenet: fix increase rx_read_ptr
From: Florian Fainelli @ 2014-10-07 17:01 UTC (permalink / raw)
To: Jaedon Shin; +Cc: netdev
In-Reply-To: <A2B48C43-2DFE-4D2F-9950-B834AE469F23@me.com>
On 10/06/2014 11:40 PM, Jaedon Shin wrote:
> This patch fixes the previous commit b629be5c8399d7c423b92135eb43a86c924d1cbc ("net: bcmgenet: check harder for out of memory conditions").
>
> The previous commit has a problem that gets invalid dma_length_status by increased rx_read_ptr. And it should be increased after all goto refill.
Good catch, I have some other GENET and SYSTEMPORT fixes for net that I
will submit shortly. Thank you!
>
>
>> On Oct 7, 2014, at 1:45 AM, Florian Fainelli <f.fainelli@gmail.com> wrote:
>>
>> On 10/05/2014 08:05 PM, Jaedon Shin wrote:
>>> The rx_read_ptr must increase after using it.
>>
>> Your commit message is too terse, you need to explain why you think the
>> current code is bad, and how your patch is fixing it.
>>
>> One possible thing that I see is that we might be off by one in how we
>> use the enet_cb versus how we read the HW packet descriptor.
>>
>>>
>>> Signed-off-by: Jaedon Shin <jaedon.shin@gmail.com>
>>> ---
>>> drivers/net/ethernet/broadcom/genet/bcmgenet.c | 6 +++---
>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
>>> index 5cc9cae..b47db5e 100644
>>> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
>>> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
>>> @@ -1282,9 +1282,6 @@ static unsigned int bcmgenet_desc_rx(struct bcmgenet_priv *priv,
>>>
>>> rxpktprocessed++;
>>>
>>> - priv->rx_read_ptr++;
>>> - priv->rx_read_ptr &= (priv->num_rx_bds - 1);
>>> -
>>> /* We do not have a backing SKB, so we do not have a
>>> * corresponding DMA mapping for this incoming packet since
>>> * bcmgenet_rx_refill always either has both skb and mapping or
>>> @@ -1399,6 +1396,9 @@ refill:
>>> err = bcmgenet_rx_refill(priv, cb);
>>> if (err)
>>> netif_err(priv, rx_err, dev, "Rx refill failed\n");
>>> +
>>> + priv->rx_read_ptr++;
>>> + priv->rx_read_ptr &= (priv->num_rx_bds - 1);
>>> }
>>>
>>> return rxpktprocessed;
>>>
>>
>
^ permalink raw reply
* Re: linux-next: Tree for Oct 7 (openvswitch)
From: Randy Dunlap @ 2014-10-07 16:59 UTC (permalink / raw)
To: Stephen Rothwell, linux-next-u79uwXL29TY76Z2rM5mHXA
Cc: dev-yBygre7rU0TnMu66kgdUjQ,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20141007194454.10502188-3FnU+UHB4dNDw9hX6IcOSA@public.gmane.org>
On 10/07/14 01:44, Stephen Rothwell wrote:
> Hi all,
>
> Changes since 20141003:
>
on i386 or x86_64:
In file included from ../include/net/geneve.h:4:0,
from ../net/openvswitch/flow_netlink.c:45:
../include/net/udp_tunnel.h: In function 'udp_tunnel_handle_offloads':
../include/net/udp_tunnel.h:100:2: error: implicit declaration of function 'iptunnel_handle_offloads' [-Werror=implicit-function-declaration]
return iptunnel_handle_offloads(skb, udp_csum, type);
^
../include/net/udp_tunnel.h:100:2: warning: return makes pointer from integer without a cast [enabled by default]
--
~Randy
^ permalink raw reply
* Re: [PATCH] net: Add ndo_gso_check
From: Alexei Starovoitov @ 2014-10-07 16:50 UTC (permalink / raw)
To: Tom Herbert
Cc: Jesse Gross, Or Gerlitz, Alexander Duyck, John Fastabend,
Jeff Kirsher, David Miller, Linux Netdev List, Thomas Graf,
Pravin Shelar, Andy Zhou
On Mon, Oct 6, 2014 at 5:17 PM, Tom Herbert <therbert@google.com> wrote:
> On Mon, Oct 6, 2014 at 3:33 PM, Jesse Gross <jesse@nicira.com> wrote:
>
>> I have no disagreement with trying to be generic across protocols. I'm
>> just not convinced that it is a realistic plan. It's obvious that it
>> is not doable today nor will be it be in the next generation of NICs
>> (which are guaranteed to add support for new protocols). Furthermore,
>> there will be more advanced stuff coming in the future that I think
>> will be difficult or impossible to make protocol agnostic. Rather than
>> pretending that this doesn't exist or will never happen, it's better
>> focus on how to integrating it cleanly.
>
> Sorry, but I don't understand how supporting a new protocols in a
> device for the purposes of returning CHECKSUM_UNNECESSARY is better or
> easier to implement than just returning CHECKSUM_COMPLETE. Same thing
> for trying to use NETIF_F_IP_CSUM with encapsulation rather than
> NETIF_F_HW_CSUM. I'm not a hardware guy, so it's possible I'm missing
> something obvious...
it's definitely more difficult to properly implement
CHECKSUM_UNNECESSARY in HW, but it's worth it.
CHECKSUM_COMPLETE is a burden on software. Old NICs
used to do that, but overhead of recomputing csum for every
step of packet parsing and header modifications is too high.
sw routers, bridges and < L4 networking devices are
simpler and faster with CHECKSUM_UNNECESSARY.
^ permalink raw reply
* Re: [PATCH v2 net-next] net: phy: adjust fixed_phy_register() return value
From: Petri Gynther @ 2014-10-07 16:48 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Florian Fainelli, Thomas Petazzoni
In-Reply-To: <20141007.000754.1011567266866832634.davem@davemloft.net>
Hi David,
On Mon, Oct 6, 2014 at 9:07 PM, David Miller <davem@davemloft.net> wrote:
> From: Petri Gynther <pgynther@google.com>
> Date: Mon, 6 Oct 2014 11:38:30 -0700 (PDT)
>
>> Adjust fixed_phy_register() to return struct phy_device *, so that
>> it becomes easy to use fixed PHYs without device tree support:
>>
>> phydev = fixed_phy_register(PHY_POLL, &fixed_phy_status, NULL);
>> fixed_phy_set_link_update(phydev, fixed_phy_link_update);
>> phy_connect_direct(netdev, phydev, handler_fn, phy_interface);
>>
>> This change is a prerequisite for modifying bcmgenet driver to work
>> without a device tree on Broadcom's MIPS-based 7xxx platforms.
>>
>> Signed-off-by: Petri Gynther <pgynther@google.com>
>
> If the caller gets this 'phy' pointer and does something with it,
> something seems amiss.
>
> We don't hold an extra reference to the 'phy' object for the caller,
> so another thread of control can unregister it and kill that last
> reference and therefore free it up.
>
> I think to be legitimate, you have to hold an extra reference on
> 'phy' for the caller. And now that means that code paths that
> don't need to do anything with 'phy' now will need to release
> that reference.
I'm not sure if I understand your comment. The caller of
fixed_phy_register() now gets the pointer to the phydev created by
get_phy_device(). What other thread is aware of this pointer and how
could they free it? Isn't the caller of fixed_phy_register()
exclusively in charge of the created phydev?
I'm trying to make fixed_phy_device() more convenient to use, so that
drivers don't need to make separate calls to fixed_phy_add() +
get_phy_device() + phy_device_register().
-- Petri
^ permalink raw reply
* RE: [net-next PATCH v1 1/3] net: sched: af_packet support for direct ring access
From: Zhou, Danny @ 2014-10-07 16:46 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Willem de Bruijn, John Fastabend, Daniel Borkmann,
Florian Westphal, gerlitz.or@gmail.com, Hannes Frederic Sowa,
Network Development, Ronciak, John, Amir Vadai, Eric Dumazet,
David S. Miller
In-Reply-To: <CAADnVQJf8jtRr3xBdSOt_X188Xkkg0EnxVd+gh6ruTMjc9jOHg@mail.gmail.com>
> -----Original Message-----
> From: Alexei Starovoitov [mailto:alexei.starovoitov@gmail.com]
> Sent: Wednesday, October 08, 2014 12:33 AM
> To: Zhou, Danny
> Cc: Willem de Bruijn; John Fastabend; Daniel Borkmann; Florian Westphal; gerlitz.or@gmail.com; Hannes Frederic Sowa; Network
> Development; Ronciak, John; Amir Vadai; Eric Dumazet; David S. Miller
> Subject: Re: [net-next PATCH v1 1/3] net: sched: af_packet support for direct ring access
>
> On Tue, Oct 7, 2014 at 8:21 AM, Zhou, Danny <danny.zhou@intel.com> wrote:
> >
> > As a master driver, the NIC kernel driver still takes control of flow director as a ethtool backend. Generally,
> > not all queues are initialized and used by NIC kernel driver, which reports actually used rx/tx numbers to stacks.
> > Before splitting off certain queues, if you want use ethtool to direct traffics to those unused queues,
> > ethtool reports invalid argument. Once certain stack-unaware queues are allocated for user space slave driver,
> > ethtool allows directing packets to them as the NIC driver maintains a data struct about which queues are visible
> > and used by kernel, which are used by user space.
>
> this whole thing sounds like it's a way to let DPDK apps share physical
> interfaces with kernel, so that tcp-based control plane traffic
> can reach DPDK process via kernel while DPDK user space
> data path does the rest of packet processing?
> One still needs pcie register i/o and all of user space driver support
> to make it work, right?
> I guess that's great for DPDK users, but I don't think it's good for linux.
No, it is a generic NIC resource(e.g. rx/tx queue pairs) portioning mechanism among NIC kernel and user space drivers with
different performance/latency/jitter characteristics. One only needs write efficient packet rx/tx routines by taking advantage
of whatever software optimizations, to manipulate limited rx/tx queue relevant registers on PCIe I/O space. In other words, no
need to port the entire NIC driver from kernel to user space.
^ permalink raw reply
* Re: [net-next PATCH v1 1/3] net: sched: af_packet support for direct ring access
From: Alexei Starovoitov @ 2014-10-07 16:33 UTC (permalink / raw)
To: Zhou, Danny
Cc: Willem de Bruijn, John Fastabend, Daniel Borkmann,
Florian Westphal, gerlitz.or@gmail.com, Hannes Frederic Sowa,
Network Development, Ronciak, John, Amir Vadai, Eric Dumazet,
David S. Miller
On Tue, Oct 7, 2014 at 8:21 AM, Zhou, Danny <danny.zhou@intel.com> wrote:
>
> As a master driver, the NIC kernel driver still takes control of flow director as a ethtool backend. Generally,
> not all queues are initialized and used by NIC kernel driver, which reports actually used rx/tx numbers to stacks.
> Before splitting off certain queues, if you want use ethtool to direct traffics to those unused queues,
> ethtool reports invalid argument. Once certain stack-unaware queues are allocated for user space slave driver,
> ethtool allows directing packets to them as the NIC driver maintains a data struct about which queues are visible
> and used by kernel, which are used by user space.
this whole thing sounds like it's a way to let DPDK apps share physical
interfaces with kernel, so that tcp-based control plane traffic
can reach DPDK process via kernel while DPDK user space
data path does the rest of packet processing?
One still needs pcie register i/o and all of user space driver support
to make it work, right?
I guess that's great for DPDK users, but I don't think it's good for linux.
^ permalink raw reply
* [PATCH v2 net-next] net: bcmgenet: fix Tx ring priority programming
From: Petri Gynther @ 2014-10-07 16:30 UTC (permalink / raw)
To: netdev; +Cc: davem, f.fainelli
GENET MAC has three Tx ring priority registers:
- GENET_x_TDMA_PRIORITY0 for queues 0-5
- GENET_x_TDMA_PRIORITY1 for queues 6-11
- GENET_x_TDMA_PRIORITY2 for queues 12-16
Fix bcmgenet_init_multiq() to program them correctly.
Signed-off-by: Petri Gynther <pgynther@google.com>
---
drivers/net/ethernet/broadcom/genet/bcmgenet.c | 42 +++++++++++++++-----------
drivers/net/ethernet/broadcom/genet/bcmgenet.h | 2 ++
2 files changed, 27 insertions(+), 17 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index e0a6238..fff2634 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -191,8 +191,9 @@ enum dma_reg {
DMA_STATUS,
DMA_SCB_BURST_SIZE,
DMA_ARB_CTRL,
- DMA_PRIORITY,
- DMA_RING_PRIORITY,
+ DMA_PRIORITY_0,
+ DMA_PRIORITY_1,
+ DMA_PRIORITY_2,
};
static const u8 bcmgenet_dma_regs_v3plus[] = {
@@ -201,8 +202,9 @@ static const u8 bcmgenet_dma_regs_v3plus[] = {
[DMA_STATUS] = 0x08,
[DMA_SCB_BURST_SIZE] = 0x0C,
[DMA_ARB_CTRL] = 0x2C,
- [DMA_PRIORITY] = 0x30,
- [DMA_RING_PRIORITY] = 0x38,
+ [DMA_PRIORITY_0] = 0x30,
+ [DMA_PRIORITY_1] = 0x34,
+ [DMA_PRIORITY_2] = 0x38,
};
static const u8 bcmgenet_dma_regs_v2[] = {
@@ -211,8 +213,9 @@ static const u8 bcmgenet_dma_regs_v2[] = {
[DMA_STATUS] = 0x08,
[DMA_SCB_BURST_SIZE] = 0x0C,
[DMA_ARB_CTRL] = 0x30,
- [DMA_PRIORITY] = 0x34,
- [DMA_RING_PRIORITY] = 0x3C,
+ [DMA_PRIORITY_0] = 0x34,
+ [DMA_PRIORITY_1] = 0x38,
+ [DMA_PRIORITY_2] = 0x3C,
};
static const u8 bcmgenet_dma_regs_v1[] = {
@@ -220,8 +223,9 @@ static const u8 bcmgenet_dma_regs_v1[] = {
[DMA_STATUS] = 0x04,
[DMA_SCB_BURST_SIZE] = 0x0C,
[DMA_ARB_CTRL] = 0x30,
- [DMA_PRIORITY] = 0x34,
- [DMA_RING_PRIORITY] = 0x3C,
+ [DMA_PRIORITY_0] = 0x34,
+ [DMA_PRIORITY_1] = 0x38,
+ [DMA_PRIORITY_2] = 0x3C,
};
/* Set at runtime once bcmgenet version is known */
@@ -1696,7 +1700,8 @@ static void bcmgenet_init_multiq(struct net_device *dev)
{
struct bcmgenet_priv *priv = netdev_priv(dev);
unsigned int i, dma_enable;
- u32 reg, dma_ctrl, ring_cfg = 0, dma_priority = 0;
+ u32 reg, dma_ctrl, ring_cfg = 0;
+ u32 dma_priority[3] = {0, 0, 0};
if (!netif_is_multiqueue(dev)) {
netdev_warn(dev, "called with non multi queue aware HW\n");
@@ -1721,22 +1726,25 @@ static void bcmgenet_init_multiq(struct net_device *dev)
/* Configure ring as descriptor ring and setup priority */
ring_cfg |= 1 << i;
- dma_priority |= ((GENET_Q0_PRIORITY + i) <<
- (GENET_MAX_MQ_CNT + 1) * i);
dma_ctrl |= 1 << (i + DMA_RING_BUF_EN_SHIFT);
+
+ dma_priority[DMA_PRIO_REG_INDEX(i)] |=
+ ((GENET_Q0_PRIORITY + i) << DMA_PRIO_REG_SHIFT(i));
}
+ /* Set ring 16 priority and program the hardware registers */
+ dma_priority[DMA_PRIO_REG_INDEX(DESC_INDEX)] |=
+ ((GENET_Q0_PRIORITY + priv->hw_params->tx_queues) <<
+ DMA_PRIO_REG_SHIFT(DESC_INDEX));
+ bcmgenet_tdma_writel(priv, dma_priority[0], DMA_PRIORITY_0);
+ bcmgenet_tdma_writel(priv, dma_priority[1], DMA_PRIORITY_1);
+ bcmgenet_tdma_writel(priv, dma_priority[2], DMA_PRIORITY_2);
+
/* Enable rings */
reg = bcmgenet_tdma_readl(priv, DMA_RING_CFG);
reg |= ring_cfg;
bcmgenet_tdma_writel(priv, reg, DMA_RING_CFG);
- /* Use configured rings priority and set ring #16 priority */
- reg = bcmgenet_tdma_readl(priv, DMA_RING_PRIORITY);
- reg |= ((GENET_Q0_PRIORITY + priv->hw_params->tx_queues) << 20);
- reg |= dma_priority;
- bcmgenet_tdma_writel(priv, reg, DMA_PRIORITY);
-
/* Configure ring as descriptor ring and re-enable DMA if enabled */
reg = bcmgenet_tdma_readl(priv, DMA_CTRL);
reg |= dma_ctrl;
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.h b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
index 321b1db..dbf524e 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.h
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
@@ -401,6 +401,8 @@ struct bcmgenet_mib_counters {
#define DMA_ARBITER_MODE_MASK 0x03
#define DMA_RING_BUF_PRIORITY_MASK 0x1F
#define DMA_RING_BUF_PRIORITY_SHIFT 5
+#define DMA_PRIO_REG_INDEX(q) ((q) / 6)
+#define DMA_PRIO_REG_SHIFT(q) (((q) % 6) * DMA_RING_BUF_PRIORITY_SHIFT)
#define DMA_RATE_ADJ_MASK 0xFF
/* Tx/Rx Dma Descriptor common bits*/
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related
* [PATCH net-next 1/1] tipc: fix bug in multicast congestion handling
From: Jon Maloy @ 2014-10-07 16:18 UTC (permalink / raw)
To: davem
Cc: netdev, Paul Gortmaker, erik.hugne, ying.xue, maloy,
tipc-discussion, Jon Maloy
One aim of commit 50100a5e39461b2a61d6040e73c384766c29975d ("tipc:
use pseudo message to wake up sockets after link congestion") was
to handle link congestion abatement in a uniform way for both unicast
and multicast transmit. However, the latter doesn't work correctly,
and has been broken since the referenced commit was applied.
If a user now sends a burst of multicast messages that is big
enough to cause broadcast link congestion, it will be put to sleep,
and not be waked up when the congestion abates as it should be.
This has two reasons. First, the flag that is used, TIPC_WAKEUP_USERS,
is set correctly, but in the wrong field. Instead of setting it in the
'action_flags' field of the arrival node struct, it is by mistake set
in the dummy node struct that is owned by the broadcast link, where it
will never tested for. Second, we cannot use the same flag for waking
up unicast and multicast users, since the function tipc_node_unlock()
needs to pick the wakeup pseudo messages to deliver from different
queues. It must hence be able to distinguish between the two cases.
This commit solves this problem by adding a new flag
TIPC_WAKEUP_BCAST_USERS, and a new function tipc_bclink_wakeup_user().
The latter is to be called by tipc_node_unlock() when the named flag,
now set in the correct field, is encountered.
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
net/tipc/bcast.c | 14 +++++++++++++-
net/tipc/bcast.h | 2 +-
net/tipc/node.c | 5 +++++
net/tipc/node.h | 3 ++-
4 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c
index b2bbe69..b8670bf 100644
--- a/net/tipc/bcast.c
+++ b/net/tipc/bcast.c
@@ -226,6 +226,17 @@ static void bclink_retransmit_pkt(u32 after, u32 to)
}
/**
+ * tipc_bclink_wakeup_users - wake up pending users
+ *
+ * Called with no locks taken
+ */
+void tipc_bclink_wakeup_users(void)
+{
+ while (skb_queue_len(&bclink->link.waiting_sks))
+ tipc_sk_rcv(skb_dequeue(&bclink->link.waiting_sks));
+}
+
+/**
* tipc_bclink_acknowledge - handle acknowledgement of broadcast packets
* @n_ptr: node that sent acknowledgement info
* @acked: broadcast sequence # that has been acknowledged
@@ -300,7 +311,8 @@ void tipc_bclink_acknowledge(struct tipc_node *n_ptr, u32 acked)
bclink_set_last_sent();
}
if (unlikely(released && !skb_queue_empty(&bcl->waiting_sks)))
- bclink->node.action_flags |= TIPC_WAKEUP_USERS;
+ n_ptr->action_flags |= TIPC_WAKEUP_BCAST_USERS;
+
exit:
tipc_bclink_unlock();
}
diff --git a/net/tipc/bcast.h b/net/tipc/bcast.h
index 4875d95..e7b0f85 100644
--- a/net/tipc/bcast.h
+++ b/net/tipc/bcast.h
@@ -99,5 +99,5 @@ int tipc_bclink_set_queue_limits(u32 limit);
void tipc_bcbearer_sort(struct tipc_node_map *nm_ptr, u32 node, bool action);
uint tipc_bclink_get_mtu(void);
int tipc_bclink_xmit(struct sk_buff *buf);
-
+void tipc_bclink_wakeup_users(void);
#endif
diff --git a/net/tipc/node.c b/net/tipc/node.c
index 17e6378..54950ac 100644
--- a/net/tipc/node.c
+++ b/net/tipc/node.c
@@ -552,6 +552,7 @@ void tipc_node_unlock(struct tipc_node *node)
LIST_HEAD(conn_sks);
struct sk_buff_head waiting_sks;
u32 addr = 0;
+ uint flags = node->action_flags;
if (likely(!node->action_flags)) {
spin_unlock_bh(&node->lock);
@@ -572,6 +573,7 @@ void tipc_node_unlock(struct tipc_node *node)
node->action_flags &= ~TIPC_NOTIFY_NODE_UP;
addr = node->addr;
}
+ node->action_flags &= ~TIPC_WAKEUP_BCAST_USERS;
spin_unlock_bh(&node->lock);
while (!skb_queue_empty(&waiting_sks))
@@ -583,6 +585,9 @@ void tipc_node_unlock(struct tipc_node *node)
if (!list_empty(&nsub_list))
tipc_nodesub_notify(&nsub_list);
+ if (flags & TIPC_WAKEUP_BCAST_USERS)
+ tipc_bclink_wakeup_users();
+
if (addr)
tipc_named_node_up(addr);
}
diff --git a/net/tipc/node.h b/net/tipc/node.h
index 522d6f3..67513c3 100644
--- a/net/tipc/node.h
+++ b/net/tipc/node.h
@@ -59,7 +59,8 @@ enum {
TIPC_WAIT_OWN_LINKS_DOWN = (1 << 2),
TIPC_NOTIFY_NODE_DOWN = (1 << 3),
TIPC_NOTIFY_NODE_UP = (1 << 4),
- TIPC_WAKEUP_USERS = (1 << 5)
+ TIPC_WAKEUP_USERS = (1 << 5),
+ TIPC_WAKEUP_BCAST_USERS = (1 << 6)
};
/**
--
1.9.1
^ permalink raw reply related
* Re: [PATCH net-next] net: bcmgenet: fix Tx ring priority programming
From: Petri Gynther @ 2014-10-07 16:09 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Florian Fainelli
In-Reply-To: <20141006.235926.30933585433924137.davem@davemloft.net>
Hi David,
On Mon, Oct 6, 2014 at 8:59 PM, David Miller <davem@davemloft.net> wrote:
> From: Petri Gynther <pgynther@google.com>
> Date: Mon, 6 Oct 2014 17:50:01 -0700 (PDT)
>
>> @@ -1731,11 +1744,12 @@ static void bcmgenet_init_multiq(struct net_device *dev)
>> reg |= ring_cfg;
>> bcmgenet_tdma_writel(priv, reg, DMA_RING_CFG);
>>
>> - /* Use configured rings priority and set ring #16 priority */
>> - reg = bcmgenet_tdma_readl(priv, DMA_RING_PRIORITY);
>> - reg |= ((GENET_Q0_PRIORITY + priv->hw_params->tx_queues) << 20);
>> - reg |= dma_priority;
>> - bcmgenet_tdma_writel(priv, reg, DMA_PRIORITY);
>> + /* Set ring 16 priority and program the hardware registers */
>> + dma_priority[2] |=
>> + ((GENET_Q0_PRIORITY + priv->hw_params->tx_queues) << 20);
>
> Please use "<< (16 - 12) * DMA_RING_BUF_PRIORITY_SHIFT" otherwise this
> constant is magic.
>
> You might, optionally, add macros for the subtraction adjustment each
> priority register uses (0, 6, 12, respectively).
Thanks for the comments. I'm going to simplify this with a few macros.
^ permalink raw reply
* Re: [net-next PATCH v1 1/3] net: sched: af_packet support for direct ring access
From: David Miller @ 2014-10-07 16:08 UTC (permalink / raw)
To: David.Laight
Cc: willemb, john.fastabend, dborkman, fw, gerlitz.or, hannes, netdev,
john.ronciak, amirv, eric.dumazet, danny.zhou
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D174C621F@AcuExch.aculab.com>
From: David Laight <David.Laight@ACULAB.COM>
Date: Tue, 7 Oct 2014 15:59:35 +0000
> From: David
>> From: David Laight <David.Laight@ACULAB.COM>
>> Date: Tue, 7 Oct 2014 09:27:03 +0000
>>
>> > That is (probably) the only scheme that stops the application
>> > accessing random parts of physical memory.
>>
>> I don't know where this claim keeps coming from, it's false.
>>
>> The application has to attach memory to the ring, and then the
>> ring can only refer to that memory for the duration of the
>> session.
>>
>> There is no way that the user can program the address field of the
>> descriptors to point at arbitrary physical memory locations.
>>
>> There is protection and control.
>
> I got the impression that the application was directly writing the ring
> structure that the ethernet mac hardware uses to describe tx and rx buffers.
> (ie they are mapped read-write into userspace).
> Unless you have a system where you can limit the physical memory
> ranges accessible to the mac hardware, I don't see how you can stop
> the application putting rogue values into the ring.
>
> Clearly I'm missing something in my quick read of the change.
No, I think I misunderstood, and apparently the Mellanox driver allows
the user to crap into arbitrary physical memory too.
All of this garbage must get fixed and this feature is a non-starter
until there is control over the memory the rings can point ti.
^ permalink raw reply
* RE: [net-next PATCH v1 1/3] net: sched: af_packet support for direct ring access
From: Zhou, Danny @ 2014-10-07 16:06 UTC (permalink / raw)
To: Fastabend, John R, Willem de Bruijn
Cc: John Fastabend, Daniel Borkmann, Florian Westphal,
gerlitz.or@gmail.com, Hannes Frederic Sowa, Network Development,
Ronciak, John, Amir Vadai, Eric Dumazet
In-Reply-To: <54340CEB.204@intel.com>
> -----Original Message-----
> From: Fastabend, John R
> Sent: Tuesday, October 07, 2014 11:55 PM
> To: Willem de Bruijn; Zhou, Danny
> Cc: John Fastabend; Daniel Borkmann; Florian Westphal; gerlitz.or@gmail.com; Hannes Frederic Sowa; Network Development;
> Ronciak, John; Amir Vadai; Eric Dumazet
> Subject: Re: [net-next PATCH v1 1/3] net: sched: af_packet support for direct ring access
>
> On 10/07/2014 08:46 AM, Willem de Bruijn wrote:
> >>>> Typically in an af_packet interface a packet_type handler is
> >>>> registered and used to filter traffic to the socket and do other
> >>>> things such as fan out traffic to multiple sockets. In this case the
> >>>> networking stack is being bypassed so this code is not run. So the
> >>>> hardware must push the correct traffic to the queues obtained from
> >>>> the ndo callback ndo_split_queue_pairs().
> >>>
> >>> Why does the interface work at the level of queue_pairs instead of
> >>> individual queues?
> >>
> >> The user mode "slave" driver(I call it slave driver because it is only responsible for packet I/O
> >> on certain queue pairs) needs at least take over one rx queue and one tx queue for ingress and
> >> egress traffics respectively, although the flow director only applies to ingress traffics.
> >
> > That requirement of co-allocation is absent in existing packet
> > rings. Many applications only receive or transmit. For
> > receive-only, it would even be possible to map descriptor
> > rings read-only, if the kernel remains responsible for posting
> > buffers -- but I see below that that is not the case, so that's
> > not very relevant here.
> >
> > Still, some workloads want asymmetric sets of rx and tx rings.
> > For instance, instead of using RSS, a process may want to
> > receive on as few rings as possible, load balance across
> > workers in software, but still give each worker thread its own
> > private transmit ring.
> >
>
> We can build this into the interface by having the setsockopt
> provide both the number of tx rings and number of rx rings. It
> might not be immediately available in any drivers because at
> least ixgbe is pretty dependent on tx/rx pairing.
>
> I would have to look through the other drivers to see how
> much work it would be to support this on them. If I can't find
> a good candidate we might leave it out until we can fix up the
> drivers.
>
Fully agreed. Unlike ixgbe, DPDK supports asymmetric sets of rx and tx rings. It should be easily enabled
if user space requests those free and available rx/tx queues, but it would cause a lot of troubles if certain
requested queues have been occupied by ixgbe, as it breaks existing rx/tx pairing.
> >
> >>>
> >>>> /* Get the layout of ring space offset, page_sz, cnt */
> >>>> getsockopt(fd, SOL_PACKET, PACKET_DEV_QPAIR_MAP_REGION_INFO,
> >>>> &info, &optlen);
> >>>>
> >>>> /* request some queues from the driver */
> >>>> setsockopt(fd, SOL_PACKET, PACKET_RXTX_QPAIRS_SPLIT,
> >>>> &qpairs_info, sizeof(qpairs_info));
> >>>>
> >>>> /* if we let the driver pick us queues learn which queues
> >>>> * we were given
> >>>> */
> >>>> getsockopt(fd, SOL_PACKET, PACKET_RXTX_QPAIRS_SPLIT,
> >>>> &qpairs_info, sizeof(qpairs_info));
> >>>
> >>> If ethtool -U is used to steer traffic to a specific descriptor queue,
> >>> then the setsockopt can pass the exact id of that queue and there
> >>> is no need for a getsockopt follow-up.
> >>
> >> Very good point, it supports pass "-1" as queue id(following by number of qpairs needed) via
> >> setsockopt to af_packet and NIC kernel driver to ask the driver dynamically allocate free and
> >> available qpairs for this socket, so getsockopt() is needed to return the actually assigned queue pair indexes.
> >> Initially, we had a implementation that calls getsockopt once and af_packet treats qpairs_info
> >> as a IN/OUT parameter, but it is semantic wrong, so we think above implementation is most suitable.
> >> But I agree with you, if setsockopt can pass the exact id with a valid queue pair index, there is no need
> >> to call getsocketopt.
> >
> > One step further would be to move the entire configuration behind
> > the packet socket interface. It's perhaps out of scope of this patch,
> > but the difference between using `ethtool -U` and passing the same
> > expression through the packet socket is that in the latter case the
> > kernel can automatically rollback the configuration change when the
> > process dies.
> >
>
> hmm might be interesting I think this is a follow on path to
> investigate after the initial support.
>
> >>
> >>>
> >>>> /* And mmap queue pairs to user space */
> >>>> mmap(NULL, info.tp_dev_bar_sz, PROT_READ | PROT_WRITE,
> >>>> MAP_SHARED, fd, 0);
> >>>
> >>> How will packet data be mapped and how will userspace translate
> >>> from paddr to vaddr? Is the goal to maintain long lived mappings
> >>> and instruct drivers to allocate from this restricted range (to
> >>> avoid per-packet system calls and vma operations)?
> >>>
> >>
> >> Once qpairs split-off is done, the user space driver, as a slave driver, will re-initialize those queues
> >> completely in user space by using paddr(in the case of DPDK, vaddr of DPDK used huge pages
> >> are translated to paddr) to fill in the packet descriptors.
> >
> > Ah, userspace is responsible for posting buffers and translation
> > from vaddr to paddr is straightforward. Yes that makes sense.
> > --
> > To unsubscribe from this list: send the line "unsubscribe netdev" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
^ permalink raw reply
* Re: [PATCH] net: fec: fix regression on i.MX28 introduced by rx_copybreak support
From: Sergei Shtylyov @ 2014-10-07 16:07 UTC (permalink / raw)
To: Lothar Waßmann, netdev
Cc: David S. Miller, Russell King, Frank Li, Fabio Estevam,
linux-kernel
In-Reply-To: <1412687977-11742-1-git-send-email-LW@KARO-electronics.de>
Hello.
On 10/07/2014 05:19 PM, Lothar Waßmann wrote:
> commit 1b7bde6d659d ("net: fec: implement rx_copybreak to improve rx performance")
> introduced a regression for i.MX28. The swap_buffer() function doing
> the endian conversion of the received data on i.MX28 may access memory
> beyond the actual packet size in the DMA buffer. fec_enet_copybreak()
> does not copy those bytes, so that the last bytes of a packet may be
> filled with invalid data after swapping.
> This will likely lead to checksum errors on received packets.
> E.g. when trying to mount an NFS rootfs:
> UDP: bad checksum. From 192.168.1.225:111 to 192.168.100.73:44662 ulen 36
> Do the byte swapping and copying to the new skb in one go if
> necessary.
> Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> ---
> drivers/net/ethernet/freescale/fec_main.c | 25 +++++++++++++++++++++----
> 1 file changed, 21 insertions(+), 4 deletions(-)
> diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
> index 87975b5..eaaebad 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
[...]
> @@ -1348,7 +1360,7 @@ fec_enet_new_rxbdp(struct net_device *ndev, struct bufdesc *bdp, struct sk_buff
> }
>
> static bool fec_enet_copybreak(struct net_device *ndev, struct sk_buff **skb,
> - struct bufdesc *bdp, u32 length)
> + struct bufdesc *bdp, u32 length, int swap)
'bool swap' perhaps?
> @@ -1393,6 +1408,7 @@ fec_enet_rx_queue(struct net_device *ndev, int budget, u16 queue_id)
> u16 vlan_tag;
> int index = 0;
> bool is_copybreak;
> + bool need_swap = id_entry->driver_data & FEC_QUIRK_SWAP_FRAME;
... especially talking this into account...
WBR, Sergei
^ permalink raw reply
* Re: [net-next PATCH v1 1/3] net: sched: af_packet support for direct ring access
From: David Miller @ 2014-10-07 16:05 UTC (permalink / raw)
To: danny.zhou
Cc: willemb, john.fastabend, dborkman, fw, gerlitz.or, hannes, netdev,
john.ronciak, amirv, eric.dumazet
In-Reply-To: <DFDF335405C17848924A094BC35766CF0A953308@SHSMSX104.ccr.corp.intel.com>
From: "Zhou, Danny" <danny.zhou@intel.com>
Date: Tue, 7 Oct 2014 15:21:15 +0000
> Once qpairs split-off is done, the user space driver, as a slave
> driver, will re-initialize those queues completely in user space by
> using paddr(in the case of DPDK, vaddr of DPDK used huge pages are
> translated to paddr) to fill in the packet descriptors. As of
> security concern raised in previous discussion, the reason we
> think(BTW, correct me if I am wrong) af_packet is most suitable is
> because only user application with root permission is allowed to
> successfully split-off queue pairs and mmap a small window of PCIe
> I/O space to user space, so concern regarding "device can DMA
> from/to any arbitrary physical memory." is not that big. As all user
> space device drivers based on UIO mechanism has the same concern
> issue, VFIO adds protection but it is based on IOMMU which is
> specific to Intel silicons.
Wait a second.
If there is no memory protection performed I'm not merging this.
I thought the user has to associate a fixed pool of memory to the
queueus, the kernel attaches that memory, and then the user cannot
modify the addresses _AT_ _ALL_.
If the user can modify the addresses in the descriptors and make
the chip crap on random memory, this is a non-starter.
Sorry.
^ permalink raw reply
* Re: [iproute2 1/1] RFC: obsolete direct invocation of police
From: Stephen Hemminger @ 2014-10-07 16:04 UTC (permalink / raw)
To: Jamal Hadi Salim; +Cc: netdev, xiyou.wangcong, john.r.fastabend
In-Reply-To: <5433CF39.5030202@mojatatu.com>
On Tue, 07 Oct 2014 07:32:09 -0400
Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> On 10/06/14 13:01, Stephen Hemminger wrote:
>
> > I think iproute utilities needs to accept the old syntax and warn about
> > deprecated syntax use. Later (like 2yr +) the code can be removed.
> >
> > The old syntax can be removed from all documentation and help messages
> > now though.
>
> Ok, so ignore that one patch. I will send another one.
> Fair to put a date for when the obsoletion notice went out?
> Example:
> "As of October 10, 2014 this syntax is obsolete. Please use instead ..."
> Maybe also mention when the approximate cutoff date is.
Don't put a date in, it either will look silly, or will be ignored.
Make message a short direct one liner.
^ permalink raw reply
* RE: [net-next PATCH v1 1/3] net: sched: af_packet support for direct ring access
From: David Laight @ 2014-10-07 15:59 UTC (permalink / raw)
To: 'David Miller'
Cc: willemb@google.com, john.fastabend@gmail.com, dborkman@redhat.com,
fw@strlen.de, gerlitz.or@gmail.com, hannes@stressinduktion.org,
netdev@vger.kernel.org, john.ronciak@intel.com,
amirv@mellanox.com, eric.dumazet@gmail.com, danny.zhou@intel.com
In-Reply-To: <20141007.114341.1417644866461362364.davem@davemloft.net>
From: David
> From: David Laight <David.Laight@ACULAB.COM>
> Date: Tue, 7 Oct 2014 09:27:03 +0000
>
> > That is (probably) the only scheme that stops the application
> > accessing random parts of physical memory.
>
> I don't know where this claim keeps coming from, it's false.
>
> The application has to attach memory to the ring, and then the
> ring can only refer to that memory for the duration of the
> session.
>
> There is no way that the user can program the address field of the
> descriptors to point at arbitrary physical memory locations.
>
> There is protection and control.
I got the impression that the application was directly writing the ring
structure that the ethernet mac hardware uses to describe tx and rx buffers.
(ie they are mapped read-write into userspace).
Unless you have a system where you can limit the physical memory
ranges accessible to the mac hardware, I don't see how you can stop
the application putting rogue values into the ring.
Clearly I'm missing something in my quick read of the change.
David
^ permalink raw reply
* Re: [net-next PATCH v1 1/3] net: sched: af_packet support for direct ring access
From: John Fastabend @ 2014-10-07 15:55 UTC (permalink / raw)
To: Willem de Bruijn, Zhou, Danny
Cc: John Fastabend, Daniel Borkmann, Florian Westphal,
gerlitz.or@gmail.com, Hannes Frederic Sowa, Network Development,
Ronciak, John, Amir Vadai, Eric Dumazet
In-Reply-To: <CA+FuTSe=vo1-Xpk+318SNc-mCH_c0WQadXo3usiA_dRBNx_fEQ@mail.gmail.com>
On 10/07/2014 08:46 AM, Willem de Bruijn wrote:
>>>> Typically in an af_packet interface a packet_type handler is
>>>> registered and used to filter traffic to the socket and do other
>>>> things such as fan out traffic to multiple sockets. In this case the
>>>> networking stack is being bypassed so this code is not run. So the
>>>> hardware must push the correct traffic to the queues obtained from
>>>> the ndo callback ndo_split_queue_pairs().
>>>
>>> Why does the interface work at the level of queue_pairs instead of
>>> individual queues?
>>
>> The user mode "slave" driver(I call it slave driver because it is only responsible for packet I/O
>> on certain queue pairs) needs at least take over one rx queue and one tx queue for ingress and
>> egress traffics respectively, although the flow director only applies to ingress traffics.
>
> That requirement of co-allocation is absent in existing packet
> rings. Many applications only receive or transmit. For
> receive-only, it would even be possible to map descriptor
> rings read-only, if the kernel remains responsible for posting
> buffers -- but I see below that that is not the case, so that's
> not very relevant here.
>
> Still, some workloads want asymmetric sets of rx and tx rings.
> For instance, instead of using RSS, a process may want to
> receive on as few rings as possible, load balance across
> workers in software, but still give each worker thread its own
> private transmit ring.
>
We can build this into the interface by having the setsockopt
provide both the number of tx rings and number of rx rings. It
might not be immediately available in any drivers because at
least ixgbe is pretty dependent on tx/rx pairing.
I would have to look through the other drivers to see how
much work it would be to support this on them. If I can't find
a good candidate we might leave it out until we can fix up the
drivers.
>
>>>
>>>> /* Get the layout of ring space offset, page_sz, cnt */
>>>> getsockopt(fd, SOL_PACKET, PACKET_DEV_QPAIR_MAP_REGION_INFO,
>>>> &info, &optlen);
>>>>
>>>> /* request some queues from the driver */
>>>> setsockopt(fd, SOL_PACKET, PACKET_RXTX_QPAIRS_SPLIT,
>>>> &qpairs_info, sizeof(qpairs_info));
>>>>
>>>> /* if we let the driver pick us queues learn which queues
>>>> * we were given
>>>> */
>>>> getsockopt(fd, SOL_PACKET, PACKET_RXTX_QPAIRS_SPLIT,
>>>> &qpairs_info, sizeof(qpairs_info));
>>>
>>> If ethtool -U is used to steer traffic to a specific descriptor queue,
>>> then the setsockopt can pass the exact id of that queue and there
>>> is no need for a getsockopt follow-up.
>>
>> Very good point, it supports pass "-1" as queue id(following by number of qpairs needed) via
>> setsockopt to af_packet and NIC kernel driver to ask the driver dynamically allocate free and
>> available qpairs for this socket, so getsockopt() is needed to return the actually assigned queue pair indexes.
>> Initially, we had a implementation that calls getsockopt once and af_packet treats qpairs_info
>> as a IN/OUT parameter, but it is semantic wrong, so we think above implementation is most suitable.
>> But I agree with you, if setsockopt can pass the exact id with a valid queue pair index, there is no need
>> to call getsocketopt.
>
> One step further would be to move the entire configuration behind
> the packet socket interface. It's perhaps out of scope of this patch,
> but the difference between using `ethtool -U` and passing the same
> expression through the packet socket is that in the latter case the
> kernel can automatically rollback the configuration change when the
> process dies.
>
hmm might be interesting I think this is a follow on path to
investigate after the initial support.
>>
>>>
>>>> /* And mmap queue pairs to user space */
>>>> mmap(NULL, info.tp_dev_bar_sz, PROT_READ | PROT_WRITE,
>>>> MAP_SHARED, fd, 0);
>>>
>>> How will packet data be mapped and how will userspace translate
>>> from paddr to vaddr? Is the goal to maintain long lived mappings
>>> and instruct drivers to allocate from this restricted range (to
>>> avoid per-packet system calls and vma operations)?
>>>
>>
>> Once qpairs split-off is done, the user space driver, as a slave driver, will re-initialize those queues
>> completely in user space by using paddr(in the case of DPDK, vaddr of DPDK used huge pages
>> are translated to paddr) to fill in the packet descriptors.
>
> Ah, userspace is responsible for posting buffers and translation
> from vaddr to paddr is straightforward. Yes that makes sense.
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [net-next PATCH v1 1/3] net: sched: af_packet support for direct ring access
From: Willem de Bruijn @ 2014-10-07 15:46 UTC (permalink / raw)
To: Zhou, Danny
Cc: John Fastabend, Daniel Borkmann, Florian Westphal,
gerlitz.or@gmail.com, Hannes Frederic Sowa, Network Development,
Ronciak, John, Amir Vadai, Eric Dumazet
In-Reply-To: <DFDF335405C17848924A094BC35766CF0A953308@SHSMSX104.ccr.corp.intel.com>
>> > Typically in an af_packet interface a packet_type handler is
>> > registered and used to filter traffic to the socket and do other
>> > things such as fan out traffic to multiple sockets. In this case the
>> > networking stack is being bypassed so this code is not run. So the
>> > hardware must push the correct traffic to the queues obtained from
>> > the ndo callback ndo_split_queue_pairs().
>>
>> Why does the interface work at the level of queue_pairs instead of
>> individual queues?
>
> The user mode "slave" driver(I call it slave driver because it is only responsible for packet I/O
> on certain queue pairs) needs at least take over one rx queue and one tx queue for ingress and
> egress traffics respectively, although the flow director only applies to ingress traffics.
That requirement of co-allocation is absent in existing packet
rings. Many applications only receive or transmit. For
receive-only, it would even be possible to map descriptor
rings read-only, if the kernel remains responsible for posting
buffers -- but I see below that that is not the case, so that's
not very relevant here.
Still, some workloads want asymmetric sets of rx and tx rings.
For instance, instead of using RSS, a process may want to
receive on as few rings as possible, load balance across
workers in software, but still give each worker thread its own
private transmit ring.
>>
>> > /* Get the layout of ring space offset, page_sz, cnt */
>> > getsockopt(fd, SOL_PACKET, PACKET_DEV_QPAIR_MAP_REGION_INFO,
>> > &info, &optlen);
>> >
>> > /* request some queues from the driver */
>> > setsockopt(fd, SOL_PACKET, PACKET_RXTX_QPAIRS_SPLIT,
>> > &qpairs_info, sizeof(qpairs_info));
>> >
>> > /* if we let the driver pick us queues learn which queues
>> > * we were given
>> > */
>> > getsockopt(fd, SOL_PACKET, PACKET_RXTX_QPAIRS_SPLIT,
>> > &qpairs_info, sizeof(qpairs_info));
>>
>> If ethtool -U is used to steer traffic to a specific descriptor queue,
>> then the setsockopt can pass the exact id of that queue and there
>> is no need for a getsockopt follow-up.
>
> Very good point, it supports pass "-1" as queue id(following by number of qpairs needed) via
> setsockopt to af_packet and NIC kernel driver to ask the driver dynamically allocate free and
> available qpairs for this socket, so getsockopt() is needed to return the actually assigned queue pair indexes.
> Initially, we had a implementation that calls getsockopt once and af_packet treats qpairs_info
> as a IN/OUT parameter, but it is semantic wrong, so we think above implementation is most suitable.
> But I agree with you, if setsockopt can pass the exact id with a valid queue pair index, there is no need
> to call getsocketopt.
One step further would be to move the entire configuration behind
the packet socket interface. It's perhaps out of scope of this patch,
but the difference between using `ethtool -U` and passing the same
expression through the packet socket is that in the latter case the
kernel can automatically rollback the configuration change when the
process dies.
>
>>
>> > /* And mmap queue pairs to user space */
>> > mmap(NULL, info.tp_dev_bar_sz, PROT_READ | PROT_WRITE,
>> > MAP_SHARED, fd, 0);
>>
>> How will packet data be mapped and how will userspace translate
>> from paddr to vaddr? Is the goal to maintain long lived mappings
>> and instruct drivers to allocate from this restricted range (to
>> avoid per-packet system calls and vma operations)?
>>
>
> Once qpairs split-off is done, the user space driver, as a slave driver, will re-initialize those queues
> completely in user space by using paddr(in the case of DPDK, vaddr of DPDK used huge pages
> are translated to paddr) to fill in the packet descriptors.
Ah, userspace is responsible for posting buffers and translation
from vaddr to paddr is straightforward. Yes that makes sense.
^ permalink raw reply
* Re: [net-next PATCH v1 1/3] net: sched: af_packet support for direct ring access
From: David Miller @ 2014-10-07 15:43 UTC (permalink / raw)
To: David.Laight
Cc: willemb, john.fastabend, dborkman, fw, gerlitz.or, hannes, netdev,
john.ronciak, amirv, eric.dumazet, danny.zhou
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D174C5C35@AcuExch.aculab.com>
From: David Laight <David.Laight@ACULAB.COM>
Date: Tue, 7 Oct 2014 09:27:03 +0000
> That is (probably) the only scheme that stops the application
> accessing random parts of physical memory.
I don't know where this claim keeps coming from, it's false.
The application has to attach memory to the ring, and then the
ring can only refer to that memory for the duration of the
session.
There is no way that the user can program the address field of the
descriptors to point at arbitrary physical memory locations.
There is protection and control.
^ permalink raw reply
* Re: [PATCH 08/16] virtio_net: drop config_enable
From: Michael S. Tsirkin @ 2014-10-07 15:42 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-kernel, virtualization
In-Reply-To: <20141007.113643.1462528588440998168.davem@davemloft.net>
On Tue, Oct 07, 2014 at 11:36:43AM -0400, David Miller wrote:
> From: "Michael S. Tsirkin" <mst@redhat.com>
> Date: Tue, 7 Oct 2014 09:49:15 +0300
>
> > On Mon, Oct 06, 2014 at 03:02:38PM -0400, David Miller wrote:
> >> From: "Michael S. Tsirkin" <mst@redhat.com>
> >> Date: Sun, 5 Oct 2014 19:07:13 +0300
> >>
> >> > Now that virtio core ensures config changes don't arrive during probing,
> >> > drop config_enable flag in virtio net.
> >> > On removal, flush is now sufficient to guarantee that no change work is
> >> > queued.
> >> >
> >> > This help simplify the driver, and will allow setting DRIVER_OK earlier
> >> > without losing config change notifications.
> >> >
> >> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> >>
> >> It's hard for people on the networking side to review these changes
> >> since you haven't CC:'d them on any of the postings necessary to
> >> understand the context of the net/ and drivers/net/ changes.
> >>
> >> Please at a minimum CC: everyone on your header [PATCH 0/N] posting
> >> so we know at least at a high level what is going on, and why.
> >>
> >> Thanks.
> >
> > It's a bit tricky for large patchsets - if I add everyone to 0/N
> > then vger isn't happy with Cc list that is too large.
> >
> > What is your advice here? Cc just mailing lists on 0/N?
> >
> > FWIW this patchset is inteded for the virtio tree.
>
> CC: mailing lists and "focus" developers, a small carefully selected
> group of people who would be strongly interested in this change.
>
> I really don't understand why this is so complicated, I've never run
> into a situation where I had to CC: 200 people in my two decades of
> kernel development :-/
Will do for the next version, thanks!
^ permalink raw reply
* Re: [PATCH 08/16] virtio_net: drop config_enable
From: David Miller @ 2014-10-07 15:36 UTC (permalink / raw)
To: mst; +Cc: netdev, linux-kernel, virtualization
In-Reply-To: <20141007064915.GA2424@redhat.com>
From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Tue, 7 Oct 2014 09:49:15 +0300
> On Mon, Oct 06, 2014 at 03:02:38PM -0400, David Miller wrote:
>> From: "Michael S. Tsirkin" <mst@redhat.com>
>> Date: Sun, 5 Oct 2014 19:07:13 +0300
>>
>> > Now that virtio core ensures config changes don't arrive during probing,
>> > drop config_enable flag in virtio net.
>> > On removal, flush is now sufficient to guarantee that no change work is
>> > queued.
>> >
>> > This help simplify the driver, and will allow setting DRIVER_OK earlier
>> > without losing config change notifications.
>> >
>> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>>
>> It's hard for people on the networking side to review these changes
>> since you haven't CC:'d them on any of the postings necessary to
>> understand the context of the net/ and drivers/net/ changes.
>>
>> Please at a minimum CC: everyone on your header [PATCH 0/N] posting
>> so we know at least at a high level what is going on, and why.
>>
>> Thanks.
>
> It's a bit tricky for large patchsets - if I add everyone to 0/N
> then vger isn't happy with Cc list that is too large.
>
> What is your advice here? Cc just mailing lists on 0/N?
>
> FWIW this patchset is inteded for the virtio tree.
CC: mailing lists and "focus" developers, a small carefully selected
group of people who would be strongly interested in this change.
I really don't understand why this is so complicated, I've never run
into a situation where I had to CC: 200 people in my two decades of
kernel development :-/
^ permalink raw reply
* Re: Quota in __qdisc_run() (was: qdisc: validate skb without holding lock)
From: Jesper Dangaard Brouer @ 2014-10-07 15:26 UTC (permalink / raw)
To: Hannes Frederic Sowa
Cc: Eric Dumazet, David Miller, netdev, therbert, fw, dborkman, jhs,
alexander.duyck, john.r.fastabend, dave.taht, toke, brouer
In-Reply-To: <1412693013.4140057.176149461.0FBF6CBD@webmail.messagingengine.com>
On Tue, 07 Oct 2014 16:43:33 +0200
Hannes Frederic Sowa <hannes@stressinduktion.org> wrote:
> On Tue, Oct 7, 2014, at 15:30, Jesper Dangaard Brouer wrote:
[...]
> >
> > The basic idea is we want keep/restore the quota fairness between
> > qdisc's , that we sort of broke with commit 5772e9a346 ("qdisc: bulk
> > dequeue support for qdiscs with TCQ_F_ONETXQUEUE").
> >
[...]
>
> This needs to be:
>
> do
> ...
> while ((iskb = iskb->next))
Check, testing with this update, now.
My netperf-wrapper test with GSO=off TSO=off, looks much more stable at
keeping the 10G link fully utilized. Before, without this patch, I
could not get stable results at 10G with GSO=off TSO=off. Think this
really does address the fairness (I didn't think I would be able to
measure it).
The other cases (GSO=on,TSO=off) and (GSO=on,TSO=on) looks the same.
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Sr. Network Kernel Developer at Red Hat
Author of http://www.iptv-analyzer.org
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* RE: [net-next PATCH v1 1/3] net: sched: af_packet support for direct ring access
From: Zhou, Danny @ 2014-10-07 15:21 UTC (permalink / raw)
To: Willem de Bruijn, John Fastabend
Cc: Daniel Borkmann, Florian Westphal, gerlitz.or@gmail.com,
Hannes Frederic Sowa, Network Development, Ronciak, John,
Amir Vadai, Eric Dumazet
In-Reply-To: <CA+FuTSfwTBtZLu7CXh4bPxUtVubOvpCPo+O38BsnSiLdvV_KEA@mail.gmail.com>
> -----Original Message-----
> From: Willem de Bruijn [mailto:willemb@google.com]
> Sent: Tuesday, October 07, 2014 12:24 PM
> To: John Fastabend
> Cc: Daniel Borkmann; Florian Westphal; gerlitz.or@gmail.com; Hannes Frederic Sowa; Network Development; Ronciak, John; Amir
> Vadai; Eric Dumazet; Zhou, Danny
> Subject: Re: [net-next PATCH v1 1/3] net: sched: af_packet support for direct ring access
>
> > Supporting some way to steer traffic to a queue
> > is the _only_ hardware requirement to support the interface,
>
> I would not impose his constraint. There may be legitimate use
> cases for taking over all queues of a device. For instance, when
> this is a secondary nic that does not carry any control traffic.
For the secondary NIC that carries the data plane traffics only, you can use UIO or VFIO
to map the entire NIC's entire I/O space to user space. Then the user space poll-mode driver,
like those have been supported and open-sourced in DPDK and those supports
Mellanox/Emulex NICs but not open-sourced, can drive the NIC as a sole driver in user space.
>
> > Typically in an af_packet interface a packet_type handler is
> > registered and used to filter traffic to the socket and do other
> > things such as fan out traffic to multiple sockets. In this case the
> > networking stack is being bypassed so this code is not run. So the
> > hardware must push the correct traffic to the queues obtained from
> > the ndo callback ndo_split_queue_pairs().
>
> Why does the interface work at the level of queue_pairs instead of
> individual queues?
The user mode "slave" driver(I call it slave driver because it is only responsible for packet I/O
on certain queue pairs) needs at least take over one rx queue and one tx queue for ingress and
egress traffics respectively, although the flow director only applies to ingress traffics.
>
> > /* Get the layout of ring space offset, page_sz, cnt */
> > getsockopt(fd, SOL_PACKET, PACKET_DEV_QPAIR_MAP_REGION_INFO,
> > &info, &optlen);
> >
> > /* request some queues from the driver */
> > setsockopt(fd, SOL_PACKET, PACKET_RXTX_QPAIRS_SPLIT,
> > &qpairs_info, sizeof(qpairs_info));
> >
> > /* if we let the driver pick us queues learn which queues
> > * we were given
> > */
> > getsockopt(fd, SOL_PACKET, PACKET_RXTX_QPAIRS_SPLIT,
> > &qpairs_info, sizeof(qpairs_info));
>
> If ethtool -U is used to steer traffic to a specific descriptor queue,
> then the setsockopt can pass the exact id of that queue and there
> is no need for a getsockopt follow-up.
Very good point, it supports pass "-1" as queue id(following by number of qpairs needed) via
setsockopt to af_packet and NIC kernel driver to ask the driver dynamically allocate free and
available qpairs for this socket, so getsockopt() is needed to return the actually assigned queue pair indexes.
Initially, we had a implementation that calls getsockopt once and af_packet treats qpairs_info
as a IN/OUT parameter, but it is semantic wrong, so we think above implementation is most suitable.
But I agree with you, if setsockopt can pass the exact id with a valid queue pair index, there is no need
to call getsocketopt.
>
> > /* And mmap queue pairs to user space */
> > mmap(NULL, info.tp_dev_bar_sz, PROT_READ | PROT_WRITE,
> > MAP_SHARED, fd, 0);
>
> How will packet data be mapped and how will userspace translate
> from paddr to vaddr? Is the goal to maintain long lived mappings
> and instruct drivers to allocate from this restricted range (to
> avoid per-packet system calls and vma operations)?
>
Once qpairs split-off is done, the user space driver, as a slave driver, will re-initialize those queues
completely in user space by using paddr(in the case of DPDK, vaddr of DPDK used huge pages
are translated to paddr) to fill in the packet descriptors.
As of security concern raised in previous discussion, the reason we think(BTW, correct me if I am wrong)
af_packet is most suitable is because only user application with root permission is allowed to successfully
split-off queue pairs and mmap a small window of PCIe I/O space to user space, so concern regarding "device
can DMA from/to any arbitrary physical memory." is not that big. As all user space device drivers based on
UIO mechanism has the same concern issue, VFIO adds protection but it is based on IOMMU which is
specific to Intel silicons.
> For throughput-oriented workloads, the syscall overhead
> involved in kicking the nic (on tx, or for increasing the ring
> consumer index on rx) can be amortized. And the operation
> can perhaps piggy-back on interrupts or other events
> (as long as interrupts are not disabled for full userspace
> polling). Latency would be harder to satisfy while maintaining
> some kernel policy enforcement. An extreme solution
> uses an asynchronously busy polling kernel worker thread
> (at high cycle cost, so acceptable for few workloads).
>
> When keeping the kernel in the loop, it is possible to do
> some basic sanity checking and transparently translate between
> vaddr and paddr, even when exposing the hardware descriptors
> directly. Though at this point it may be just as cheap to expose
> an idealized virtualized descriptor format and copy fields between
> that and device descriptors.
>
> One assumption underlying exposing the hardware descriptors
> is that they are quire similar between devices. How true is this
> in the context of formats that span multiple descriptors?
>
Packet descriptors format varies for different vendors. On Intel NICs, 1G/10G/40G NICs have
totally different formats. Even a same Intel 10G/40G NIC supports at least 2 different descriptor
formats. IMHO, the idea behind those patches intent to skip descriptor difference among devices,
as it just maps certain I/O space pages to user space and user space "slave" NIC driver can handle
it using different descriptor struct based on vendor/device ID. But I am open to add support of generic
packet descriptor format description, per David M' suggestion.
> > + * int (*ndo_split_queue_pairs) (struct net_device *dev,
> > + * unsigned int qpairs_start_from,
> > + * unsigned int qpairs_num,
> > + * struct sock *sk)
> > + * Called to request a set of queues from the driver to be
> > + * handed to the callee for management. After this returns the
> > + * driver will not use the queues.
>
> Are these queues also taken out of ethtool management, or is
> this equivalent to taking removing them from the rss set with
> ethtool -X?
As a master driver, the NIC kernel driver still takes control of flow director as a ethtool backend. Generally,
not all queues are initialized and used by NIC kernel driver, which reports actually used rx/tx numbers to stacks.
Before splitting off certain queues, if you want use ethtool to direct traffics to those unused queues,
ethtool reports invalid argument. Once certain stack-unaware queues are allocated for user space slave driver,
ethtool allows directing packets to them as the NIC driver maintains a data struct about which queues are visible
and used by kernel, which are used by user space.
^ permalink raw reply
* Re: Quota in __qdisc_run() (was: qdisc: validate skb without holding lock)
From: Eric Dumazet @ 2014-10-07 15:06 UTC (permalink / raw)
To: Hannes Frederic Sowa
Cc: Jesper Dangaard Brouer, David Miller, netdev, therbert, fw,
dborkman, jhs, alexander.duyck, john.r.fastabend, dave.taht, toke
In-Reply-To: <1412694080.11091.131.camel@edumazet-glaptop2.roam.corp.google.com>
On Tue, 2014-10-07 at 08:01 -0700, Eric Dumazet wrote:
> Quota was a packet quota, which was quite irrelevant if segmentation had
> to be done, so I would just let the dequeue be done so that we benefit
> from optimal xmit_more.
And it also is better to allow receiver to get full LRO/GRO aggregation,
if we do not break GSO train in multiple parts.
^ permalink raw reply
* Re: [PATCH v2 1/1] tcp: fixing TLP's FIN recovery
From: Josh Hunt @ 2014-10-07 15:03 UTC (permalink / raw)
To: David Miller
Cc: per.hurtig, Eric Dumazet, panweiping3, nanditad, netdev,
anna.brunstrom, mohammad.rajiullah, ncardwell, sergei.shtylyov
In-Reply-To: <20140612.110611.584155630422154338.davem@davemloft.net>
On Thu, Jun 12, 2014 at 1:06 PM, David Miller <davem@davemloft.net> wrote:
> From: Per Hurtig <per.hurtig@kau.se>
> Date: Thu, 12 Jun 2014 17:08:32 +0200
>
>> Fix to a problem observed when losing a FIN segment that does not
>> contain data. In such situations, TLP is unable to recover from
>> *any* tail loss and instead adds at least PTO ms to the
>> retransmission process, i.e., RTO = RTO + PTO.
>>
>> Signed-off-by: Per Hurtig <per.hurtig@kau.se>
>
> Applied, thanks.
Can we queue this up for stable? 2cd0d743b05e87 (tcp: fix
tcp_match_skb_to_sack() for unaligned SACK at end of an skb) is
already in stable and based on the changelog was put in place to fix a
case that this patch introduced:
"This was visible now because the recently simplified TLP logic in
bef1909ee3ed1c ("tcp: fixing TLP's FIN recovery") could find that 0-byte
skb at the end of the write queue, and now that we do not check that
skb's length we could send it as a TLP probe."
However, the patch to fix TLP's FIN recovery is not in -stable.
Thanks
--
Josh
^ 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