* [PATCH net-next 0/3] mlx4: CQE/EQE stride support
From: Or Gerlitz @ 2014-09-18 8:50 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Amir Vadai, Ido Shamay, Or Gerlitz
This series from Ido Shamay is intended for archs having
cache line larger then 64 bytes.
Since our CQE/EQEs are generally 64B in those systems, HW will write
twice to the same cache line consecutively, causing pipe locks due to
he hazard prevention mechanism. For elements in a cyclic buffer, writes
are consecutive, so entries smaller than a cache line should be
avoided, especially if they are written at a high rate.
Reduce consecutive writes to same cache line in CQs/EQs, by allowing the
driver to increase the distance between entries so that each will reside
in a different cache line.
Or.
Ido Shamay (3):
net/mlx4_core: Enable CQE/EQE stride support
net/mlx4_core: Cache line EQE size support
net/mlx4_en: Add mlx4_en_get_cqe helper
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 1 +
drivers/net/ethernet/mellanox/mlx4/en_rx.c | 4 +-
drivers/net/ethernet/mellanox/mlx4/en_tx.c | 4 +-
drivers/net/ethernet/mellanox/mlx4/eq.c | 30 +++++++----
drivers/net/ethernet/mellanox/mlx4/fw.c | 38 ++++++++++++++-
drivers/net/ethernet/mellanox/mlx4/fw.h | 2 +
drivers/net/ethernet/mellanox/mlx4/main.c | 61 +++++++++++++++++++++++-
drivers/net/ethernet/mellanox/mlx4/mlx4.h | 3 +
drivers/net/ethernet/mellanox/mlx4/mlx4_en.h | 6 ++
include/linux/mlx4/device.h | 11 +++-
10 files changed, 138 insertions(+), 22 deletions(-)
^ permalink raw reply
* [PATCH net-next 3/3] net/mlx4_en: Add mlx4_en_get_cqe helper
From: Or Gerlitz @ 2014-09-18 8:51 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Amir Vadai, Ido Shamay, Jack Morgenstein, Or Gerlitz
In-Reply-To: <1411030261-12145-1-git-send-email-ogerlitz@mellanox.com>
From: Ido Shamay <idos@mellanox.com>
This function derives the base address of the CQE from the CQE size,
and calculates the real CQE context segment in it from the factor
(this is like before). Before this change the code used the factor to
calculate the base address of the CQE as well.
The factor indicates in which segment of the cqe stride the cqe information
is located. For 32-byte strides, the segment is 0, and for 64 byte strides,
the segment is 1 (bytes 32..63). Using the factor was ok as long as we had
only 32 and 64 byte strides. However, with larger strides, the factor is zero,
and so cannot be used to calculate the base of the CQE.
The helper uses the same method of CQE buffer pulling made by all other
components that reads the CQE buffer (mlx4_ib driver and libmlx4).
Signed-off-by: Ido Shamay <idos@mellanox.com>
Signed-off-by: Jack Morgenstein <jackm@dev.mellanox.co.il>
Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 1 +
drivers/net/ethernet/mellanox/mlx4/en_rx.c | 4 ++--
drivers/net/ethernet/mellanox/mlx4/en_tx.c | 4 ++--
drivers/net/ethernet/mellanox/mlx4/mlx4_en.h | 6 ++++++
4 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index abddcf8..f3032fe 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -2459,6 +2459,7 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port,
}
priv->rx_ring_num = prof->rx_ring_num;
priv->cqe_factor = (mdev->dev->caps.cqe_size == 64) ? 1 : 0;
+ priv->cqe_size = mdev->dev->caps.cqe_size;
priv->mac_index = -1;
priv->msg_enable = MLX4_EN_MSG_LEVEL;
spin_lock_init(&priv->stats_lock);
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
index 14686b6..a33048e 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
@@ -671,7 +671,7 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud
* descriptor offset can be deduced from the CQE index instead of
* reading 'cqe->index' */
index = cq->mcq.cons_index & ring->size_mask;
- cqe = &cq->buf[(index << factor) + factor];
+ cqe = mlx4_en_get_cqe(cq->buf, index, priv->cqe_size) + factor;
/* Process all completed CQEs */
while (XNOR(cqe->owner_sr_opcode & MLX4_CQE_OWNER_MASK,
@@ -858,7 +858,7 @@ next:
++cq->mcq.cons_index;
index = (cq->mcq.cons_index) & ring->size_mask;
- cqe = &cq->buf[(index << factor) + factor];
+ cqe = mlx4_en_get_cqe(cq->buf, index, priv->cqe_size) + factor;
if (++polled == budget)
goto out;
}
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_tx.c b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
index bc8f51c..c44f423 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_tx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
@@ -382,7 +382,7 @@ static bool mlx4_en_process_tx_cq(struct net_device *dev,
return true;
index = cons_index & size_mask;
- cqe = &buf[(index << factor) + factor];
+ cqe = mlx4_en_get_cqe(buf, index, priv->cqe_size) + factor;
ring_index = ring->cons & size_mask;
stamp_index = ring_index;
@@ -430,7 +430,7 @@ static bool mlx4_en_process_tx_cq(struct net_device *dev,
++cons_index;
index = cons_index & size_mask;
- cqe = &buf[(index << factor) + factor];
+ cqe = mlx4_en_get_cqe(buf, index, priv->cqe_size) + factor;
}
diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
index 3de41be..e3d71c3 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
+++ b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
@@ -542,6 +542,7 @@ struct mlx4_en_priv {
unsigned max_mtu;
int base_qpn;
int cqe_factor;
+ int cqe_size;
struct mlx4_en_rss_map rss_map;
__be32 ctrl_flags;
@@ -612,6 +613,11 @@ struct mlx4_mac_entry {
struct rcu_head rcu;
};
+static inline struct mlx4_cqe *mlx4_en_get_cqe(void *buf, int idx, int cqe_sz)
+{
+ return buf + idx * cqe_sz;
+}
+
#ifdef CONFIG_NET_RX_BUSY_POLL
static inline void mlx4_en_cq_init_lock(struct mlx4_en_cq *cq)
{
--
1.7.1
^ permalink raw reply related
* [PATCH net-next 2/3] net/mlx4_core: Cache line EQE size support
From: Or Gerlitz @ 2014-09-18 8:51 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Amir Vadai, Ido Shamay, Jack Morgenstein, Or Gerlitz
In-Reply-To: <1411030261-12145-1-git-send-email-ogerlitz@mellanox.com>
From: Ido Shamay <idos@mellanox.com>
Enable mlx4 interrupt handler to work with EQE stride feature,
The feature may be enabled when cache line is bigger than 64B.
The EQE size will then be the cache line size, and the context
segment resides in [0-31] offset.
Signed-off-by: Ido Shamay <idos@mellanox.com>
Signed-off-by: Jack Morgenstein <jackm@dev.mellanox.co.il>
Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx4/eq.c | 30 +++++++++++++++++++-----------
1 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/eq.c b/drivers/net/ethernet/mellanox/mlx4/eq.c
index 2a004b3..a49c9d1 100644
--- a/drivers/net/ethernet/mellanox/mlx4/eq.c
+++ b/drivers/net/ethernet/mellanox/mlx4/eq.c
@@ -101,21 +101,24 @@ static void eq_set_ci(struct mlx4_eq *eq, int req_not)
mb();
}
-static struct mlx4_eqe *get_eqe(struct mlx4_eq *eq, u32 entry, u8 eqe_factor)
+static struct mlx4_eqe *get_eqe(struct mlx4_eq *eq, u32 entry, u8 eqe_factor,
+ u8 eqe_size)
{
/* (entry & (eq->nent - 1)) gives us a cyclic array */
- unsigned long offset = (entry & (eq->nent - 1)) * (MLX4_EQ_ENTRY_SIZE << eqe_factor);
- /* CX3 is capable of extending the EQE from 32 to 64 bytes.
- * When this feature is enabled, the first (in the lower addresses)
+ unsigned long offset = (entry & (eq->nent - 1)) * eqe_size;
+ /* CX3 is capable of extending the EQE from 32 to 64 bytes with
+ * strides of 64B,128B and 256B.
+ * When 64B EQE is used, the first (in the lower addresses)
* 32 bytes in the 64 byte EQE are reserved and the next 32 bytes
* contain the legacy EQE information.
+ * In all other cases, the first 32B contains the legacy EQE info.
*/
return eq->page_list[offset / PAGE_SIZE].buf + (offset + (eqe_factor ? MLX4_EQ_ENTRY_SIZE : 0)) % PAGE_SIZE;
}
-static struct mlx4_eqe *next_eqe_sw(struct mlx4_eq *eq, u8 eqe_factor)
+static struct mlx4_eqe *next_eqe_sw(struct mlx4_eq *eq, u8 eqe_factor, u8 size)
{
- struct mlx4_eqe *eqe = get_eqe(eq, eq->cons_index, eqe_factor);
+ struct mlx4_eqe *eqe = get_eqe(eq, eq->cons_index, eqe_factor, size);
return !!(eqe->owner & 0x80) ^ !!(eq->cons_index & eq->nent) ? NULL : eqe;
}
@@ -459,8 +462,9 @@ static int mlx4_eq_int(struct mlx4_dev *dev, struct mlx4_eq *eq)
enum slave_port_gen_event gen_event;
unsigned long flags;
struct mlx4_vport_state *s_info;
+ int eqe_size = dev->caps.eqe_size;
- while ((eqe = next_eqe_sw(eq, dev->caps.eqe_factor))) {
+ while ((eqe = next_eqe_sw(eq, dev->caps.eqe_factor, eqe_size))) {
/*
* Make sure we read EQ entry contents after we've
* checked the ownership bit.
@@ -894,8 +898,10 @@ static int mlx4_create_eq(struct mlx4_dev *dev, int nent,
eq->dev = dev;
eq->nent = roundup_pow_of_two(max(nent, 2));
- /* CX3 is capable of extending the CQE/EQE from 32 to 64 bytes */
- npages = PAGE_ALIGN(eq->nent * (MLX4_EQ_ENTRY_SIZE << dev->caps.eqe_factor)) / PAGE_SIZE;
+ /* CX3 is capable of extending the CQE/EQE from 32 to 64 bytes, with
+ * strides of 64B,128B and 256B.
+ */
+ npages = PAGE_ALIGN(eq->nent * dev->caps.eqe_size) / PAGE_SIZE;
eq->page_list = kmalloc(npages * sizeof *eq->page_list,
GFP_KERNEL);
@@ -997,8 +1003,10 @@ static void mlx4_free_eq(struct mlx4_dev *dev,
struct mlx4_cmd_mailbox *mailbox;
int err;
int i;
- /* CX3 is capable of extending the CQE/EQE from 32 to 64 bytes */
- int npages = PAGE_ALIGN((MLX4_EQ_ENTRY_SIZE << dev->caps.eqe_factor) * eq->nent) / PAGE_SIZE;
+ /* CX3 is capable of extending the CQE/EQE from 32 to 64 bytes, with
+ * strides of 64B,128B and 256B
+ */
+ int npages = PAGE_ALIGN(dev->caps.eqe_size * eq->nent) / PAGE_SIZE;
mailbox = mlx4_alloc_cmd_mailbox(dev);
if (IS_ERR(mailbox))
--
1.7.1
^ permalink raw reply related
* Re: ipsec: Remove obsolete MAX_AH_AUTH_LEN
From: Herbert Xu @ 2014-09-18 8:38 UTC (permalink / raw)
To: Steffen Klassert; +Cc: David S. Miller, netdev
In-Reply-To: <20140917045828.GA32086@gondor.apana.org.au>
Oops, should've sent this to Steffen:
While tracking down the MAX_AH_AUTH_LEN crash in an old kernel
I thought that this limit was rather arbitrary and we should
just get rid of it.
In fact it seems that we've already done all the work needed
to remove it apart from actually removing it. This limit was
there in order to limit stack usage. Since we've already
switched over to allocating scratch space using kmalloc, there
is no longer any need to limit the authentication length.
This patch kills all references to it, including the BUG_ONs
that led me here.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
diff --git a/include/net/ah.h b/include/net/ah.h
index ca95b98..4e2dfa4 100644
--- a/include/net/ah.h
+++ b/include/net/ah.h
@@ -3,9 +3,6 @@
#include <linux/skbuff.h>
-/* This is the maximum truncated ICV length that we know of. */
-#define MAX_AH_AUTH_LEN 64
-
struct crypto_ahash;
struct ah_data {
diff --git a/net/ipv4/ah4.c b/net/ipv4/ah4.c
index a2afa89..ac9a32e 100644
--- a/net/ipv4/ah4.c
+++ b/net/ipv4/ah4.c
@@ -505,8 +505,6 @@ static int ah_init_state(struct xfrm_state *x)
ahp->icv_full_len = aalg_desc->uinfo.auth.icv_fullbits/8;
ahp->icv_trunc_len = x->aalg->alg_trunc_len/8;
- BUG_ON(ahp->icv_trunc_len > MAX_AH_AUTH_LEN);
-
if (x->props.flags & XFRM_STATE_ALIGN4)
x->props.header_len = XFRM_ALIGN4(sizeof(struct ip_auth_hdr) +
ahp->icv_trunc_len);
diff --git a/net/ipv6/ah6.c b/net/ipv6/ah6.c
index 72a4930..2176893 100644
--- a/net/ipv6/ah6.c
+++ b/net/ipv6/ah6.c
@@ -713,8 +713,6 @@ static int ah6_init_state(struct xfrm_state *x)
ahp->icv_full_len = aalg_desc->uinfo.auth.icv_fullbits/8;
ahp->icv_trunc_len = x->aalg->alg_trunc_len/8;
- BUG_ON(ahp->icv_trunc_len > MAX_AH_AUTH_LEN);
-
x->props.header_len = XFRM_ALIGN8(sizeof(struct ip_auth_hdr) +
ahp->icv_trunc_len);
switch (x->props.mode) {
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index d4db6eb..8cf382b 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -333,8 +333,7 @@ static int attach_auth_trunc(struct xfrm_algo_auth **algpp, u8 *props,
algo = xfrm_aalg_get_byname(ualg->alg_name, 1);
if (!algo)
return -ENOSYS;
- if ((ualg->alg_trunc_len / 8) > MAX_AH_AUTH_LEN ||
- ualg->alg_trunc_len > algo->uinfo.auth.icv_fullbits)
+ if (ualg->alg_trunc_len > algo->uinfo.auth.icv_fullbits)
return -EINVAL;
*props = algo->desc.sadb_alg_id;
Thanks,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply related
* [PATCH net v2 2/2] openvswitch: restore OVS_FLOW_CMD_NEW notifications
From: Nicolas Dichtel @ 2014-09-18 8:31 UTC (permalink / raw)
To: pshelar; +Cc: davem, dev, netdev, Samuel Gauthier, Nicolas Dichtel
In-Reply-To: <1411029064-15376-1-git-send-email-nicolas.dichtel@6wind.com>
From: Samuel Gauthier <samuel.gauthier@6wind.com>
Since commit fb5d1e9e127a ("openvswitch: Build flow cmd netlink reply only if needed."),
the new flows are not notified to the listeners of OVS_FLOW_MCGROUP.
This commit fixes the problem by using the genl function, ie
genl_has_listerners() instead of netlink_has_listeners().
Signed-off-by: Samuel Gauthier <samuel.gauthier@6wind.com>
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
v2: add patch 1/2
net/openvswitch/datapath.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index 91d66b7e64ac..64dc864a417f 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -78,11 +78,12 @@ static const struct genl_multicast_group ovs_dp_vport_multicast_group = {
/* Check if need to build a reply message.
* OVS userspace sets the NLM_F_ECHO flag if it needs the reply. */
-static bool ovs_must_notify(struct genl_info *info,
- const struct genl_multicast_group *grp)
+static bool ovs_must_notify(struct genl_family *family, struct genl_info *info,
+ unsigned int group)
{
return info->nlhdr->nlmsg_flags & NLM_F_ECHO ||
- netlink_has_listeners(genl_info_net(info)->genl_sock, 0);
+ genl_has_listeners(family, genl_info_net(info)->genl_sock,
+ group);
}
static void ovs_notify(struct genl_family *family,
@@ -763,7 +764,7 @@ static struct sk_buff *ovs_flow_cmd_alloc_info(const struct sw_flow_actions *act
{
struct sk_buff *skb;
- if (!always && !ovs_must_notify(info, &ovs_dp_flow_multicast_group))
+ if (!always && !ovs_must_notify(&dp_flow_genl_family, info, 0))
return NULL;
skb = genlmsg_new_unicast(ovs_flow_cmd_msg_size(acts), info, GFP_KERNEL);
--
2.1.0
^ permalink raw reply related
* [PATCH net v2 1/2] genetlink: add function genl_has_listeners()
From: Nicolas Dichtel @ 2014-09-18 8:31 UTC (permalink / raw)
To: pshelar; +Cc: davem, dev, netdev, Nicolas Dichtel
In-Reply-To: <CALnjE+qqv66Zkh==bJAxBSUEkh8H-9YMnox1yF+4_BwetqERmg@mail.gmail.com>
This function is the counterpart of the function netlink_has_listeners().
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
v2: add patch 1/2
include/net/genetlink.h | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/include/net/genetlink.h b/include/net/genetlink.h
index 93695f0e22a5..af10c2cf8a1d 100644
--- a/include/net/genetlink.h
+++ b/include/net/genetlink.h
@@ -394,4 +394,12 @@ static inline int genl_set_err(struct genl_family *family, struct net *net,
return netlink_set_err(net->genl_sock, portid, group, code);
}
+static inline int genl_has_listeners(struct genl_family *family,
+ struct sock *sk, unsigned int group)
+{
+ if (WARN_ON_ONCE(group >= family->n_mcgrps))
+ return -EINVAL;
+ group = family->mcgrp_offset + group;
+ return netlink_has_listeners(sk, group);
+}
#endif /* __NET_GENERIC_NETLINK_H */
--
2.1.0
^ permalink raw reply related
* Comments regarding patch about setting netns for wireless devices
From: Vadim Kochan @ 2014-09-18 8:05 UTC (permalink / raw)
To: netdev@vger.kernel.org, linux-wireless, Emmanuel Grumbach,
John Linville, Oliver Hartkopp
Hi All,
I'd like to get some feedback about the patch:
https://patchwork.kernel.org/patch/4890451/
The idea is that currently Linux wireless device is allowed to change
the network namespace only by nl80211 API, so RTM_LINK API does
not work. So I think that it should be possible to change netns
through the RTM_LINK too as generic way to change netns,
thats why I tried to solve this in the patch.
Thanks,
^ permalink raw reply
* Re: [PATCHv2] ipv4: Do not cache routing failures due to disabled forwarding.
From: Julian Anastasov @ 2014-09-18 8:04 UTC (permalink / raw)
To: Nicolas Cavallari
Cc: netdev, David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy
In-Reply-To: <541A7EFA.8070506@green-communications.fr>
Hello,
On Thu, 18 Sep 2014, Nicolas Cavallari wrote:
> On 18/09/2014 07:17, Julian Anastasov wrote:
> >
> > Still, I fail to see how the compiler optimizes
> > the jump, 'goto local_input' still jumps to res.fi check.
> > I tried even likely() after 'local_input:" checks for
> > res.fi and !itag but the 'if (res.fi) {' block is still
> > moved below and reached with jnz. I expected likely() to
> > prefer the res.fi != NULL path.
>
> Different compiler/arch/options perhaps ? I'm using Debian's GCC
> 4.9.1-11 on amd64 with a defconfig + CONFIG_DEBUG_INFO. It does this:
I see, new compilers can do better, I'm on x86 32-bit:
# rpm -qi gcc | grep -e Version -e Date
Version : 4.8.3
Install Date: Sat 06 Sep 2014 05:56:15 PM EEST
Build Date : Tue 24 Jun 2014 10:18:10 PM EEST
# make net/ipv4/route.s
---> local_input:
.L1031:
movl -56(%ebp), %edx # res.fi, D.55132
testl %edx, %edx # D.55132
---> jump for likely(res.fi != NULL):
jne .L1219 #,
.L1095:
movb $0, -88(%ebp) #, %sfp
.L1059:
movzbl -88(%ebp), %eax # %sfp, D.55118
xorl %edx, %edx # D.55127
cmpl $0, 236(%edi) #, in_dev_129->cnf.data
pushl %eax # D.55118
movl init_net+172, %eax # init_net.loopback_dev,
setne %dl #, D.55127
xorl %ecx, %ecx #
call rt_dst_alloc #
...
---> no_route:
.L1027:
movb $7, -62(%ebp) #, res.type
movl $0, -56(%ebp) #, res.fi
#APP
# 1773 "net/ipv4/route.c" 1
incl rt_cache_stat+8 # rt_cache_stat.in_no_route
# 0 "" 2
#NO_APP
.L1214:
movl $0, -92(%ebp) #, %sfp
---> jump to local_input:
jmp .L1031 #
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply
* Re: CPU scheduler to TXQ binding? (ixgbe vs. igb)
From: Jesper Dangaard Brouer @ 2014-09-18 7:28 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Alexander Duyck, Eric Dumazet, netdev@vger.kernel.org,
Tom Herbert
In-Reply-To: <20140918085640.0815df6d@redhat.com>
On Thu, 18 Sep 2014 08:56:40 +0200
Jesper Dangaard Brouer <jbrouer@redhat.com> wrote:
> On Wed, 17 Sep 2014 07:59:51 -0700
> Alexander Duyck <alexander.h.duyck@intel.com> wrote:
>
> > On 09/17/2014 07:32 AM, Eric Dumazet wrote:
> > > On Wed, 2014-09-17 at 15:26 +0200, Jesper Dangaard Brouer wrote:
> > >> The CPU to TXQ binding behavior of ixgbe vs. igb NIC driver are
> > >> somehow different. Normally I setup NIC IRQ-to-CPU bindings 1-to-1,
> > >> with script set_irq_affinity [1].
> > >>
> > >> For forcing use of a specific HW TXQ, I normally force the CPU binding
> > >> of the process, either with "taskset" or with "netperf -T lcpu,rcpu".
> > >>
> > >> This works fine with driver ixgbe, but not with driver igb. That is
> > >> with igb, the program forced to specific CPU, can still use another
> > >> TXQ. What am I missing?
> > >>
> > >>
> > >> I'm monitoring this with both:
> > >> 1) watch -d sudo tc -s -d q ls dev ethXX
> > >> 2) https://github.com/ffainelli/bqlmon
> > >>
> > >> [1] https://github.com/netoptimizer/network-testing/blob/master/bin/set_irq_affinity
> > >
> > > Have you setup XPS ?
> > >
> > > echo 0001 >/sys/class/net/ethX/queues/tx-0/xps_cpus
> > > echo 0002 >/sys/class/net/ethX/queues/tx-1/xps_cpus
> > > echo 0004 >/sys/class/net/ethX/queues/tx-2/xps_cpus
> > > echo 0008 >/sys/class/net/ethX/queues/tx-3/xps_cpus
> > > echo 0010 >/sys/class/net/ethX/queues/tx-4/xps_cpus
> > > echo 0020 >/sys/class/net/ethX/queues/tx-5/xps_cpus
> > > echo 0040 >/sys/class/net/ethX/queues/tx-6/xps_cpus
> > > echo 0080 >/sys/class/net/ethX/queues/tx-7/xps_cpus
> > >
> > > Or something like that, depending on number of cpus and TX queues.
> > >
> >
> > That was what I was thinking as well.
> >
> > ixgbe has ATR which makes use of XPS to setup the transmit queues for a
> > 1:1 mapping. The receive side of the flow is routed back to the same Rx
> > queue through flow director mappings.
> >
> > In the case of igb it only has RSS and doesn't set a default XPS
> > configuration. So you should probably setup XPS and you might also want
> > to try and make use of RPS to try and steer receive packets since the Rx
> > queues won't match the Tx queues.
>
> After setting up XPS to CPU 1:1 binding, it works most of the time.
> Meaning, most of the traffic will go through the TXQ I've bound the
> process to, BUT some packets can still choose another TXQ (observed
> monitoring tc output and blqmon).
>
> Could this be related to the missing RPS setup?
It helped setting up RPS, but not 100%. I see small periods of packets
going out on other TXQs, and sometimes as before some heavy flow will
find its way to another TXQ.
> Can I get some hints setting up RPS?
My setup command now maps both XPS and RPS 1:1 to CPUs.
# Setup both RPS and XPS with a 1:1 binding to CPUs
export DEV=eth1 ; export NR_CPUS=11 ; \
for txq in `seq 0 $NR_CPUS` ; do \
file_xps=/sys/class/net/${DEV}/queues/tx-${txq}/xps_cpus \
file_rps=/sys/class/net/${DEV}/queues/rx-${txq}/rps_cpus \
mask=`printf %X $((1<<$txq))`
test -e $file_xps && sudo sh -c "echo $mask > $file_xps" && grep . -H $file_xps ;\
test -e $file_rps && sudo sh -c "echo $mask > $file_rps" && grep . -H $file_rps ;\
done
Output:
/sys/class/net/eth1/queues/tx-0/xps_cpus:001
/sys/class/net/eth1/queues/rx-0/rps_cpus:001
/sys/class/net/eth1/queues/tx-1/xps_cpus:002
/sys/class/net/eth1/queues/rx-1/rps_cpus:002
/sys/class/net/eth1/queues/tx-2/xps_cpus:004
/sys/class/net/eth1/queues/rx-2/rps_cpus:004
/sys/class/net/eth1/queues/tx-3/xps_cpus:008
/sys/class/net/eth1/queues/rx-3/rps_cpus:008
/sys/class/net/eth1/queues/tx-4/xps_cpus:010
/sys/class/net/eth1/queues/rx-4/rps_cpus:010
/sys/class/net/eth1/queues/tx-5/xps_cpus:020
/sys/class/net/eth1/queues/rx-5/rps_cpus:020
/sys/class/net/eth1/queues/tx-6/xps_cpus:040
/sys/class/net/eth1/queues/rx-6/rps_cpus:040
/sys/class/net/eth1/queues/tx-7/xps_cpus:080
/sys/class/net/eth1/queues/rx-7/rps_cpus:080
--
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: CPU scheduler to TXQ binding? (ixgbe vs. igb)
From: Jesper Dangaard Brouer @ 2014-09-18 6:56 UTC (permalink / raw)
To: Alexander Duyck; +Cc: Eric Dumazet, netdev@vger.kernel.org, Tom Herbert
In-Reply-To: <5419A1E7.8040109@intel.com>
On Wed, 17 Sep 2014 07:59:51 -0700
Alexander Duyck <alexander.h.duyck@intel.com> wrote:
> On 09/17/2014 07:32 AM, Eric Dumazet wrote:
> > On Wed, 2014-09-17 at 15:26 +0200, Jesper Dangaard Brouer wrote:
> >> The CPU to TXQ binding behavior of ixgbe vs. igb NIC driver are
> >> somehow different. Normally I setup NIC IRQ-to-CPU bindings 1-to-1,
> >> with script set_irq_affinity [1].
> >>
> >> For forcing use of a specific HW TXQ, I normally force the CPU binding
> >> of the process, either with "taskset" or with "netperf -T lcpu,rcpu".
> >>
> >> This works fine with driver ixgbe, but not with driver igb. That is
> >> with igb, the program forced to specific CPU, can still use another
> >> TXQ. What am I missing?
> >>
> >>
> >> I'm monitoring this with both:
> >> 1) watch -d sudo tc -s -d q ls dev ethXX
> >> 2) https://github.com/ffainelli/bqlmon
> >>
> >> [1] https://github.com/netoptimizer/network-testing/blob/master/bin/set_irq_affinity
> >
> > Have you setup XPS ?
> >
> > echo 0001 >/sys/class/net/ethX/queues/tx-0/xps_cpus
> > echo 0002 >/sys/class/net/ethX/queues/tx-1/xps_cpus
> > echo 0004 >/sys/class/net/ethX/queues/tx-2/xps_cpus
> > echo 0008 >/sys/class/net/ethX/queues/tx-3/xps_cpus
> > echo 0010 >/sys/class/net/ethX/queues/tx-4/xps_cpus
> > echo 0020 >/sys/class/net/ethX/queues/tx-5/xps_cpus
> > echo 0040 >/sys/class/net/ethX/queues/tx-6/xps_cpus
> > echo 0080 >/sys/class/net/ethX/queues/tx-7/xps_cpus
> >
> > Or something like that, depending on number of cpus and TX queues.
> >
>
> That was what I was thinking as well.
>
> ixgbe has ATR which makes use of XPS to setup the transmit queues for a
> 1:1 mapping. The receive side of the flow is routed back to the same Rx
> queue through flow director mappings.
>
> In the case of igb it only has RSS and doesn't set a default XPS
> configuration. So you should probably setup XPS and you might also want
> to try and make use of RPS to try and steer receive packets since the Rx
> queues won't match the Tx queues.
After setting up XPS to CPU 1:1 binding, it works most of the time.
Meaning, most of the traffic will go through the TXQ I've bound the
process to, BUT some packets can still choose another TXQ (observed
monitoring tc output and blqmon).
Could this be related to the missing RPS setup?
Can I get some hints setting up RPS?
--
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: [PATCH v13 net-next 07/11] bpf: verifier (add ability to receive verification log)
From: Daniel Borkmann @ 2014-09-18 6:44 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David S. Miller, Ingo Molnar, Linus Torvalds, Andy Lutomirski,
Hannes Frederic Sowa, Chema Gonzalez, Eric Dumazet,
Peter Zijlstra, Pablo Neira Ayuso, H. Peter Anvin, Andrew Morton,
Kees Cook, Linux API, Network Development, LKML
In-Reply-To: <CAMEtUuw0aDPpQhd13ssk2PDLffBJNeef9jicOnhQLH6KtJ8gsQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 09/18/2014 01:45 AM, Alexei Starovoitov wrote:
> On Wed, Sep 17, 2014 at 12:37 PM, Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org> wrote:
>>
>>> Hm, thinking out loudly ... perhaps this could be made a library problem.
>>> Such that the library which wraps the syscall needs to be aware of a
>>> marker where the initial version ends, and if the application doesn't
>>> make use of any of the new features, it would just pass in the length up
>>> to the marker as size attribute into the syscall. Similarly, if new
>>> features are always added to the end of a structure and the library
>>> truncates the overall-length after the last used member we might have
>>> a chance to load something on older kernels, haven't tried that though.
>>
>> that's a 3rd option. I think it's cleaner than 2nd, since it solves it
>> completely from user space.
>> It can even be smarter than that. If this syscall wrapper library
>> sees that newer features are used and it can workaround them:
>> it can chop size and pass older fields into the older kernel
>> and when it returns, do a workaround based on newer fields.
>
> the more I think about 'new user space + old kernel' problem,
> the more certain I am that kernel should not try to help
> user space, since most of the time it's not going to be enough,
> but additional code in kernel would need to be maintained.
>
> syscall commands and size of bpf_attr is the least of problems.
> New map_type and prog_type will be added, new helper
> functions will be available to programs.
> One would think that md5 of uapi/linux/bpf.h would be
> enough to say that user app is compatible... In reality,
> it's not. The 'state pruning' verifier optimization I've talked
> about will not change a single bit in bpf.h, but it will be
> able to recognize more programs as safe.
> A program developed on a new kernel with more
> advanced verifier will load just fine on new kernel, but
> this valid program will not load on old kernel, only because
> verifier is not smart enough. Now we would need a version
> of verifier exposed all the way to user space?!
> imo that's too much. I think for eBPF infra kernel
> should only guarantee backwards compatibility
> (old user space must work with new kernel) and that's it.
> That's what this patch is trying to do.
> Thoughts?
Sure, you will never get a full compatibility on that regard
while backwards compatibility needs to be guaranteed on the
other hand. I looked at perf_copy_attr() implementation and I
think that we should mimic it in a very similar way as it
exactly solves what we need.
For example, it will return with -EINVAL for (size > PAGE_SIZE)
and (size < PERF_ATTR_SIZE_VER0) where PAGE_SIZE has been chosen
as an arbitrary hard upper limit where it is believed that it will
never grow beyond that large limit in future.
So this is a more loose constraint than what we currently do,
that is, -EINVAL on (size > sizeof(attr)) where attr is the
currently known size of a specific kernel. That would at least
be a start, you won't be able to cover everything though, but
it would allow to address the issue raised when running with
a basic feature set.
^ permalink raw reply
* Re: [PATCHv2] ipv4: Do not cache routing failures due to disabled forwarding.
From: Nicolas Cavallari @ 2014-09-18 6:43 UTC (permalink / raw)
To: Julian Anastasov
Cc: netdev, David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy
In-Reply-To: <alpine.LFD.2.11.1409180749570.4152@ja.home.ssi.bg>
On 18/09/2014 07:17, Julian Anastasov wrote:
>
> Hello,
>
> On Mon, 15 Sep 2014, Nicolas Cavallari wrote:
>
>> 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>
>
> Looks good to me,
>
> Reviewed-by: Julian Anastasov <ja@ssi.bg>
>
> Still, I fail to see how the compiler optimizes
> the jump, 'goto local_input' still jumps to res.fi check.
> I tried even likely() after 'local_input:" checks for
> res.fi and !itag but the 'if (res.fi) {' block is still
> moved below and reached with jnz. I expected likely() to
> prefer the res.fi != NULL path.
Different compiler/arch/options perhaps ? I'm using Debian's GCC
4.9.1-11 on amd64 with a defconfig + CONFIG_DEBUG_INFO. It does this:
.LBE2727:
.loc 1 1800 0
movb $7, -102(%rbp) #, res.type
.loc 1 1801 0
movq $0, -96(%rbp) #, res.fi
xorl %ecx, %ecx # D.60248
.loc 1 1653 0
xorl %r12d, %r12d # flags
.LVL737:
.loc 1 1749 0
xorl %r15d, %r15d # do_cache
.LVL738:
jmp .L784 #
where .L784 is :
.loc 1 1762 0
movl 324(%r11), %edi # in_dev_74->cnf.data,
xorl %esi, %esi # D.60250
.LVL601:
movq %r10, -128(%rbp) # skb, %sfp
testl %edi, %edi #
movq 264(%r14), %rdi # _75->loopback_dev, _75->loopback_dev
setne %sil #, D.60250
xorl %edx, %edx #
.LVL602:
call rt_dst_alloc #
As for the res.fi and itag check in local_input, it jumps only when the
test fails.
^ permalink raw reply
* Re: [PATCH] [trivial] wireless:rtlwifi: Fix typo in rtl wifi drivers
From: Larry Finger @ 2014-09-18 5:45 UTC (permalink / raw)
To: trivial-DgEjT+Ai2ygdnm+yROfE0A,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA, John W Linville
Cc: Masanari Iida, netdev-u79uwXL29TY76Z2rM5mHXA,
davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <1411010992-9999-1-git-send-email-standby24x7-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On 09/17/2014 10:29 PM, Masanari Iida wrote:
> This patch fix spelling typo "sleeped" in printk, found in
> multiple rtlwifi drivers.
>
> Signed-off-by: Masanari Iida <standby24x7-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> drivers/net/wireless/rtlwifi/rtl8188ee/phy.c | 2 +-
> drivers/net/wireless/rtlwifi/rtl8192ce/phy.c | 2 +-
> drivers/net/wireless/rtlwifi/rtl8192cu/phy.c | 2 +-
> drivers/net/wireless/rtlwifi/rtl8192de/phy.c | 2 +-
> drivers/net/wireless/rtlwifi/rtl8192se/phy.c | 2 +-
> drivers/net/wireless/rtlwifi/rtl8723ae/phy.c | 2 +-
> drivers/net/wireless/rtlwifi/rtl8723be/phy.c | 2 +-
> 7 files changed, 7 insertions(+), 7 deletions(-)
John,
Please do not apply this patch for two reasons. (1) It would interfere with the
major changes that I will be sending in the next few days. (2) I have been
favoring "sleeping" over "slept" as a substitute for "sleeped". If I change my
mind, I will resubmit this patch with Masanari as the author.
Thanks,
Larry
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCHv2] ipv4: Do not cache routing failures due to disabled forwarding.
From: Julian Anastasov @ 2014-09-18 5:17 UTC (permalink / raw)
To: Nicolas Cavallari
Cc: netdev, David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy
In-Reply-To: <1410776893-5284-1-git-send-email-nicolas.cavallari@green-communications.fr>
Hello,
On Mon, 15 Sep 2014, Nicolas Cavallari wrote:
> 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>
Looks good to me,
Reviewed-by: Julian Anastasov <ja@ssi.bg>
Still, I fail to see how the compiler optimizes
the jump, 'goto local_input' still jumps to res.fi check.
I tried even likely() after 'local_input:" checks for
res.fi and !itag but the 'if (res.fi) {' block is still
moved below and reached with jnz. I expected likely() to
prefer the res.fi != NULL path.
> ---
> v2: simplify patch using julian anastasov's suggestion.
>
> net/ipv4/route.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> index 234a43e..b09fda8 100644
> --- a/net/ipv4/route.c
> +++ b/net/ipv4/route.c
> @@ -1798,6 +1798,7 @@ local_input:
> no_route:
> RT_CACHE_STAT_INC(in_no_route);
> res.type = RTN_UNREACHABLE;
> + res.fi = NULL;
> goto local_input;
>
> /*
> --
> 2.1.0
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply
* Re: Figuring out how vti works
From: Joe M @ 2014-09-18 5:08 UTC (permalink / raw)
To: Steffen Klassert; +Cc: netdev@vger.kernel.org
In-Reply-To: <CAHjjW16qdT6RWuJaUd1c6k39RgcPtfsyF=V-dVowW_2UwhF_dw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1558 bytes --]
Hello Steffen,
Checking further with printk's, I can see
vti_tunnel_init being called for both ip_vti0 and vtil tunnels. But,
when vti_tunnel_xmit is called, it is called with ip_vti0 tunnel and
not the vtil tunnel. I am not sure if I am setting the route wrong. I
saw a comment that vti_tunnel_xmit is called by dev_queue_xmit
function. I could not figure out what dev_queue_xmit does though.
master# echo "1" | sudo tee /proc/sys/net/ipv4/ip_forward
1
master# modprobe ip_vti
master# ipsec start
Starting strongSwan 5.2.0 IPsec [starter]...
master# ip tunnel add vtil mode vti local 192.168.0.11 remote 192.168.1.232 ikey 1 okey 1
master# ip link set vtil up
master# sleep 10
master# ip route add 192.168.1.0/24 dev vtil
master# ip route list
default via 192.168.0.1 dev enp4s0 metric 202
127.0.0.0/8 dev lo scope host
192.168.0.0/24 dev enp4s0 proto kernel scope link src 192.168.0.11 metric 202
192.168.1.0/24 dev vtil scope link
ip link list
.
.
.
13: ip_vti0@NONE: <NOARP,UP,LOWER_UP> mtu 1428 qdisc noqueue state UNKNOWN mode DEFAULT group default
link/ipip 0.0.0.0 brd 0.0.0.0
14: vtil@NONE: <POINTOPOINT,NOARP,UP,LOWER_UP> mtu 1428 qdisc noqueue state UNKNOWN mode DEFAULT group default
link/ipip 192.168.0.11 peer 192.168.1.232
master# ip tunnel list
ip_vti0: ip/ip remote any local any ttl inherit nopmtudisc key 0
vtil: ip/ip remote 192.168.1.232 local 192.168.0.11 ttl inherit key 1
Just wanted to check if you have any suggestions to go about debugging
this.
Thanks
Joe
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCHv6 net-next 1/3] sunvnet: upgrade to VIO protocol version 1.6
From: Raghuram Kothakota @ 2014-09-18 4:23 UTC (permalink / raw)
To: David L Stevens; +Cc: David Miller, netdev
In-Reply-To: <541A2316.5010603@oracle.com>
On Sep 17, 2014, at 5:11 PM, David L Stevens <david.stevens@oracle.com> wrote:
> This patch upgrades the sunvnet driver to support VIO protocol version 1.6.
> In particular, it adds per-port MTU negotiation, allowing MTUs other than
> ETH_FRAMELEN with ports using newer VIO protocol versions.
>
> Signed-off-by: David L Stevens <david.stevens@oracle.com>
> ---
> arch/sparc/include/asm/vio.h | 44 ++++++++++++++-
> arch/sparc/kernel/viohs.c | 14 ++++-
> drivers/net/ethernet/sun/sunvnet.c | 104 +++++++++++++++++++++++++++++------
> drivers/net/ethernet/sun/sunvnet.h | 3 +
> 4 files changed, 143 insertions(+), 22 deletions(-)
>
> diff --git a/arch/sparc/include/asm/vio.h b/arch/sparc/include/asm/vio.h
> index 5c0ebe7..107d4a4 100644
> --- a/arch/sparc/include/asm/vio.h
> +++ b/arch/sparc/include/asm/vio.h
> @@ -65,6 +65,7 @@ struct vio_dring_register {
> u16 options;
> #define VIO_TX_DRING 0x0001
> #define VIO_RX_DRING 0x0002
> +#define VIO_RX_DRING_DATA 0x0004
> u16 resv;
> u32 num_cookies;
> struct ldc_trans_cookie cookies[0];
> @@ -80,6 +81,8 @@ struct vio_dring_unregister {
> #define VIO_PKT_MODE 0x01 /* Packet based transfer */
> #define VIO_DESC_MODE 0x02 /* In-band descriptors */
> #define VIO_DRING_MODE 0x03 /* Descriptor rings */
> +/* in vers >= 1.2, VIO_DRING_MODE is 0x04 and transfer mode is a bitmask */
> +#define VIO_NEW_DRING_MODE 0x04
>
> struct vio_dring_data {
> struct vio_msg_tag tag;
> @@ -209,10 +212,20 @@ struct vio_net_attr_info {
> u8 addr_type;
> #define VNET_ADDR_ETHERMAC 0x01
> u16 ack_freq;
> - u32 resv1;
> + u8 plnk_updt;
> +#define PHYSLINK_UPDATE_NONE 0x00
> +#define PHYSLINK_UPDATE_STATE 0x01
> +#define PHYSLINK_UPDATE_STATE_ACK 0x02
> +#define PHYSLINK_UPDATE_STATE_NACK 0x03
> + u8 options;
> + u16 resv1;
> u64 addr;
> u64 mtu;
> - u64 resv2[3];
> + u16 cflags;
> +#define VNET_LSO_IPV4_CAPAB 0x0001
> + u16 ipv4_lso_maxlen;
> + u32 resv2;
> + u64 resv3[2];
> };
>
> #define VNET_NUM_MCAST 7
> @@ -370,6 +383,33 @@ struct vio_driver_state {
> struct vio_driver_ops *ops;
> };
>
> +static inline bool vio_version_before(struct vio_driver_state *vio,
> + u16 major, u16 minor)
> +{
> + u32 have = (u32)vio->ver.major << 16 | vio->ver.minor;
> + u32 want = (u32)major << 16 | minor;
> +
> + return have < want;
> +}
> +
> +static inline bool vio_version_after(struct vio_driver_state *vio,
> + u16 major, u16 minor)
> +{
> + u32 have = (u32)vio->ver.major << 16 | vio->ver.minor;
> + u32 want = (u32)major << 16 | minor;
> +
> + return have > want;
> +}
> +
> +static inline bool vio_version_after_eq(struct vio_driver_state *vio,
> + u16 major, u16 minor)
> +{
> + u32 have = (u32)vio->ver.major << 16 | vio->ver.minor;
> + u32 want = (u32)major << 16 | minor;
> +
> + return have >= want;
> +}
> +
> #define viodbg(TYPE, f, a...) \
> do { if (vio->debug & VIO_DEBUG_##TYPE) \
> printk(KERN_INFO "vio: ID[%lu] " f, \
> diff --git a/arch/sparc/kernel/viohs.c b/arch/sparc/kernel/viohs.c
> index f8e7dd5..7ef081a 100644
> --- a/arch/sparc/kernel/viohs.c
> +++ b/arch/sparc/kernel/viohs.c
> @@ -426,6 +426,13 @@ static int process_dreg_info(struct vio_driver_state *vio,
> if (vio->dr_state & VIO_DR_STATE_RXREG)
> goto send_nack;
>
> + /* v1.6 and higher, ACK with desired, supported mode, or NACK */
> + if (vio_version_after_eq(vio, 1, 6)) {
> + if (!(pkt->options & VIO_TX_DRING))
> + goto send_nack;
> + pkt->options = VIO_TX_DRING;
> + }
> +
I forget to send this comment in my previous email. The above function is
common to all clients, that includes vdisk. The vdisk today doesn't use any
version number above 1.2 or 1.3, so it probably won't impact it, but this
special handling doesn't seem to belong in the common code.
-Raghuram
> BUG_ON(vio->desc_buf);
>
> vio->desc_buf = kzalloc(pkt->descr_size, GFP_ATOMIC);
> @@ -453,8 +460,11 @@ static int process_dreg_info(struct vio_driver_state *vio,
> pkt->tag.stype = VIO_SUBTYPE_ACK;
> pkt->dring_ident = ++dr->ident;
>
> - viodbg(HS, "SEND DRING_REG ACK ident[%llx]\n",
> - (unsigned long long) pkt->dring_ident);
> + viodbg(HS, "SEND DRING_REG ACK ident[%llx] "
> + "ndesc[%u] dsz[%u] opt[0x%x] ncookies[%u]\n",
> + (unsigned long long) pkt->dring_ident,
> + pkt->num_descr, pkt->descr_size, pkt->options,
> + pkt->num_cookies);
>
> len = (sizeof(*pkt) +
> (dr->ncookies * sizeof(struct ldc_trans_cookie)));
> diff --git a/drivers/net/ethernet/sun/sunvnet.c b/drivers/net/ethernet/sun/sunvnet.c
> index 763cdfc..a9638c1 100644
> --- a/drivers/net/ethernet/sun/sunvnet.c
> +++ b/drivers/net/ethernet/sun/sunvnet.c
> @@ -15,6 +15,7 @@
> #include <linux/ethtool.h>
> #include <linux/etherdevice.h>
> #include <linux/mutex.h>
> +#include <linux/if_vlan.h>
>
> #include <asm/vio.h>
> #include <asm/ldc.h>
> @@ -41,6 +42,7 @@ static int __vnet_tx_trigger(struct vnet_port *port, u32 start);
>
> /* Ordered from largest major to lowest */
> static struct vio_version vnet_versions[] = {
> + { .major = 1, .minor = 6 },
> { .major = 1, .minor = 0 },
> };
>
> @@ -67,6 +69,7 @@ static int vnet_send_attr(struct vio_driver_state *vio)
> struct vnet_port *port = to_vnet_port(vio);
> struct net_device *dev = port->vp->dev;
> struct vio_net_attr_info pkt;
> + int framelen = ETH_FRAME_LEN;
> int i;
>
> memset(&pkt, 0, sizeof(pkt));
> @@ -74,19 +77,41 @@ static int vnet_send_attr(struct vio_driver_state *vio)
> pkt.tag.stype = VIO_SUBTYPE_INFO;
> pkt.tag.stype_env = VIO_ATTR_INFO;
> pkt.tag.sid = vio_send_sid(vio);
> - pkt.xfer_mode = VIO_DRING_MODE;
> + if (vio_version_before(vio, 1, 2))
> + pkt.xfer_mode = VIO_DRING_MODE;
> + else
> + pkt.xfer_mode = VIO_NEW_DRING_MODE;
> pkt.addr_type = VNET_ADDR_ETHERMAC;
> pkt.ack_freq = 0;
> for (i = 0; i < 6; i++)
> pkt.addr |= (u64)dev->dev_addr[i] << ((5 - i) * 8);
> - pkt.mtu = ETH_FRAME_LEN;
> + if (vio_version_after(vio, 1, 3)) {
> + if (port->rmtu) {
> + port->rmtu = min(VNET_MAXPACKET, port->rmtu);
> + pkt.mtu = port->rmtu;
> + } else {
> + port->rmtu = VNET_MAXPACKET;
> + pkt.mtu = port->rmtu;
> + }
> + if (vio_version_after_eq(vio, 1, 6))
> + pkt.options = VIO_TX_DRING;
> + } else if (vio_version_before(vio, 1, 3)) {
> + pkt.mtu = framelen;
> + } else { /* v1.3 */
> + pkt.mtu = framelen + VLAN_HLEN;
> + }
> +
> + pkt.plnk_updt = PHYSLINK_UPDATE_NONE;
> + pkt.cflags = 0;
>
> viodbg(HS, "SEND NET ATTR xmode[0x%x] atype[0x%x] addr[%llx] "
> - "ackfreq[%u] mtu[%llu]\n",
> + "ackfreq[%u] plnk_updt[0x%02x] opts[0x%02x] mtu[%llu] "
> + "cflags[0x%04x] lso_max[%u]\n",
> pkt.xfer_mode, pkt.addr_type,
> - (unsigned long long) pkt.addr,
> - pkt.ack_freq,
> - (unsigned long long) pkt.mtu);
> + (unsigned long long)pkt.addr,
> + pkt.ack_freq, pkt.plnk_updt, pkt.options,
> + (unsigned long long)pkt.mtu, pkt.cflags, pkt.ipv4_lso_maxlen);
> +
>
> return vio_ldc_send(vio, &pkt, sizeof(pkt));
> }
> @@ -94,18 +119,52 @@ static int vnet_send_attr(struct vio_driver_state *vio)
> static int handle_attr_info(struct vio_driver_state *vio,
> struct vio_net_attr_info *pkt)
> {
> - viodbg(HS, "GOT NET ATTR INFO xmode[0x%x] atype[0x%x] addr[%llx] "
> - "ackfreq[%u] mtu[%llu]\n",
> + struct vnet_port *port = to_vnet_port(vio);
> + u64 localmtu;
> + u8 xfer_mode;
> +
> + viodbg(HS, "GOT NET ATTR xmode[0x%x] atype[0x%x] addr[%llx] "
> + "ackfreq[%u] plnk_updt[0x%02x] opts[0x%02x] mtu[%llu] "
> + " (rmtu[%llu]) cflags[0x%04x] lso_max[%u]\n",
> pkt->xfer_mode, pkt->addr_type,
> - (unsigned long long) pkt->addr,
> - pkt->ack_freq,
> - (unsigned long long) pkt->mtu);
> + (unsigned long long)pkt->addr,
> + pkt->ack_freq, pkt->plnk_updt, pkt->options,
> + (unsigned long long)pkt->mtu, port->rmtu, pkt->cflags,
> + pkt->ipv4_lso_maxlen);
>
> pkt->tag.sid = vio_send_sid(vio);
>
> - if (pkt->xfer_mode != VIO_DRING_MODE ||
> + xfer_mode = pkt->xfer_mode;
> + /* for version < 1.2, VIO_DRING_MODE = 0x3 and no bitmask */
> + if (vio_version_before(vio, 1, 2) && xfer_mode == VIO_DRING_MODE)
> + xfer_mode = VIO_NEW_DRING_MODE;
> +
> + /* MTU negotiation:
> + * < v1.3 - ETH_FRAME_LEN exactly
> + * > v1.3 - MIN(pkt.mtu, VNET_MAXPACKET, port->rmtu) and change
> + * pkt->mtu for ACK
> + * = v1.3 - ETH_FRAME_LEN + VLAN_HLEN exactly
> + */
> + if (vio_version_before(vio, 1, 3)) {
> + localmtu = ETH_FRAME_LEN;
> + } else if (vio_version_after(vio, 1, 3)) {
> + localmtu = port->rmtu ? port->rmtu : VNET_MAXPACKET;
> + localmtu = min(pkt->mtu, localmtu);
> + pkt->mtu = localmtu;
> + } else { /* v1.3 */
> + localmtu = ETH_FRAME_LEN + VLAN_HLEN;
> + }
> + port->rmtu = localmtu;
> +
> + /* for version >= 1.6, ACK packet mode we support */
> + if (vio_version_after_eq(vio, 1, 6)) {
> + pkt->xfer_mode = VIO_NEW_DRING_MODE;
> + pkt->options = VIO_TX_DRING;
> + }
> +
> + if (!(xfer_mode | VIO_NEW_DRING_MODE) ||
> pkt->addr_type != VNET_ADDR_ETHERMAC ||
> - pkt->mtu != ETH_FRAME_LEN) {
> + pkt->mtu != localmtu) {
> viodbg(HS, "SEND NET ATTR NACK\n");
>
> pkt->tag.stype = VIO_SUBTYPE_NACK;
> @@ -114,7 +173,14 @@ static int handle_attr_info(struct vio_driver_state *vio,
>
> return -ECONNRESET;
> } else {
> - viodbg(HS, "SEND NET ATTR ACK\n");
> + viodbg(HS, "SEND NET ATTR ACK xmode[0x%x] atype[0x%x] "
> + "addr[%llx] ackfreq[%u] plnk_updt[0x%02x] opts[0x%02x] "
> + "mtu[%llu] (rmtu[%llu]) cflags[0x%04x] lso_max[%u]\n",
> + pkt->xfer_mode, pkt->addr_type,
> + (unsigned long long)pkt->addr,
> + pkt->ack_freq, pkt->plnk_updt, pkt->options,
> + (unsigned long long)pkt->mtu, port->rmtu, pkt->cflags,
> + pkt->ipv4_lso_maxlen);
>
> pkt->tag.stype = VIO_SUBTYPE_ACK;
>
> @@ -210,7 +276,7 @@ static int vnet_rx_one(struct vnet_port *port, unsigned int len,
> int err;
>
> err = -EMSGSIZE;
> - if (unlikely(len < ETH_ZLEN || len > ETH_FRAME_LEN)) {
> + if (unlikely(len < ETH_ZLEN || len > port->rmtu)) {
> dev->stats.rx_length_errors++;
> goto out_dropped;
> }
> @@ -555,8 +621,10 @@ static void vnet_event(void *arg, int event)
> vio_link_state_change(vio, event);
> spin_unlock_irqrestore(&vio->lock, flags);
>
> - if (event == LDC_EVENT_RESET)
> + if (event == LDC_EVENT_RESET) {
> + port->rmtu = 0;
> vio_port_up(vio);
> + }
> return;
> }
>
> @@ -1048,8 +1116,8 @@ static int vnet_port_alloc_tx_bufs(struct vnet_port *port)
> void *dring;
>
> for (i = 0; i < VNET_TX_RING_SIZE; i++) {
> - void *buf = kzalloc(ETH_FRAME_LEN + 8, GFP_KERNEL);
> - int map_len = (ETH_FRAME_LEN + 7) & ~7;
> + void *buf = kzalloc(VNET_MAXPACKET + 8, GFP_KERNEL);
> + int map_len = (VNET_MAXPACKET + 7) & ~7;
>
> err = -ENOMEM;
> if (!buf)
> diff --git a/drivers/net/ethernet/sun/sunvnet.h b/drivers/net/ethernet/sun/sunvnet.h
> index da49337..986e04b 100644
> --- a/drivers/net/ethernet/sun/sunvnet.h
> +++ b/drivers/net/ethernet/sun/sunvnet.h
> @@ -11,6 +11,7 @@
> */
> #define VNET_TX_TIMEOUT (5 * HZ)
>
> +#define VNET_MAXPACKET 1518ULL /* ETH_FRAMELEN + VLAN_HDR */
> #define VNET_TX_RING_SIZE 512
> #define VNET_TX_WAKEUP_THRESH(dr) ((dr)->pending / 4)
>
> @@ -44,6 +45,8 @@ struct vnet_port {
> u32 stop_rx_idx;
> bool stop_rx;
> bool start_cons;
> +
> + u64 rmtu;
> };
>
> static inline struct vnet_port *to_vnet_port(struct vio_driver_state *vio)
> --
> 1.7.1
>
> --
> 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: [PATCHv6 net-next 2/3] sunvnet: allow admin to set sunvnet MTU
From: Raghuram Kothakota @ 2014-09-18 4:19 UTC (permalink / raw)
To: David L Stevens; +Cc: David Miller, netdev
In-Reply-To: <541A22FC.20407@oracle.com>
On Sep 17, 2014, at 5:10 PM, David L Stevens <david.stevens@oracle.com> wrote:
> This patch allows an admin to set the MTU on a sunvnet device to arbitrary
> values between the minimum (68) and maximum (65535) IPv4 packet sizes.
>
> Signed-off-by: David L Stevens <david.stevens@oracle.com>
> ---
> arch/sparc/kernel/ldc.c | 2 +-
> drivers/net/ethernet/sun/sunvnet.c | 7 +++++--
> drivers/net/ethernet/sun/sunvnet.h | 6 ++++--
> 3 files changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/arch/sparc/kernel/ldc.c b/arch/sparc/kernel/ldc.c
> index 66dacd5..0af28b9 100644
> --- a/arch/sparc/kernel/ldc.c
> +++ b/arch/sparc/kernel/ldc.c
> @@ -2159,7 +2159,7 @@ int ldc_map_single(struct ldc_channel *lp,
> state.pte_idx = (base - iommu->page_table);
> state.nc = 0;
> fill_cookies(&state, (pa & PAGE_MASK), (pa & ~PAGE_MASK), len);
> - BUG_ON(state.nc != 1);
> + BUG_ON(state.nc > ncookies);
>
> return state.nc;
> }
> diff --git a/drivers/net/ethernet/sun/sunvnet.c b/drivers/net/ethernet/sun/sunvnet.c
> index a9638c1..5e64e60 100644
> --- a/drivers/net/ethernet/sun/sunvnet.c
> +++ b/drivers/net/ethernet/sun/sunvnet.c
> @@ -791,6 +791,9 @@ static int vnet_start_xmit(struct sk_buff *skb, struct net_device *dev)
> if (unlikely(!port))
> goto out_dropped;
>
> + if (skb->len > port->rmtu)
> + goto out_dropped;
> +
> spin_lock_irqsave(&port->vio.lock, flags);
>
> dr = &port->vio.drings[VIO_DRIVER_TX_RING];
> @@ -1038,7 +1041,7 @@ static void vnet_set_rx_mode(struct net_device *dev)
>
> static int vnet_change_mtu(struct net_device *dev, int new_mtu)
> {
> - if (new_mtu != ETH_DATA_LEN)
> + if (new_mtu < 68 || new_mtu > 65535)
> return -EINVAL;
>
FYI, LDoms manager provides the maximum MTU of packets allowed for
a vnet device in the "virtual-device" MD node corresponding to the vnet device.
The virtual switch is expected to enforce that size for packets that go through
it. Ideally we want a Guest OS to honor that MTU setting as well, which is probably
not the case here. LDoms manager doesn't allow setting more than 16K today,
I guess this code is ignoring this for Guest to Guest communication.
-Raghuram
> dev->mtu = new_mtu;
> @@ -1131,7 +1134,7 @@ static int vnet_port_alloc_tx_bufs(struct vnet_port *port)
> }
>
> err = ldc_map_single(port->vio.lp, buf, map_len,
> - port->tx_bufs[i].cookies, 2,
> + port->tx_bufs[i].cookies, VNET_MAXCOOKIES,
> (LDC_MAP_SHADOW |
> LDC_MAP_DIRECT |
> LDC_MAP_RW));
> diff --git a/drivers/net/ethernet/sun/sunvnet.h b/drivers/net/ethernet/sun/sunvnet.h
> index 986e04b..a39a801 100644
> --- a/drivers/net/ethernet/sun/sunvnet.h
> +++ b/drivers/net/ethernet/sun/sunvnet.h
> @@ -11,7 +11,7 @@
> */
> #define VNET_TX_TIMEOUT (5 * HZ)
>
> -#define VNET_MAXPACKET 1518ULL /* ETH_FRAMELEN + VLAN_HDR */
> +#define VNET_MAXPACKET (65535ULL + ETH_HLEN + VLAN_HLEN)
> #define VNET_TX_RING_SIZE 512
> #define VNET_TX_WAKEUP_THRESH(dr) ((dr)->pending / 4)
>
> @@ -21,10 +21,12 @@
> */
> #define VNET_PACKET_SKIP 6
>
> +#define VNET_MAXCOOKIES (VNET_MAXPACKET/PAGE_SIZE + 1)
> +
> struct vnet_tx_entry {
> void *buf;
> unsigned int ncookies;
> - struct ldc_trans_cookie cookies[2];
> + struct ldc_trans_cookie cookies[VNET_MAXCOOKIES];
> };
>
> struct vnet;
> --
> 1.7.1
>
> --
> 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: [PATCHv6 net-next 1/3] sunvnet: upgrade to VIO protocol version 1.6
From: Raghuram Kothakota @ 2014-09-18 4:09 UTC (permalink / raw)
To: David L Stevens; +Cc: David Miller, netdev
In-Reply-To: <541A2316.5010603@oracle.com>
On Sep 17, 2014, at 5:11 PM, David L Stevens <david.stevens@oracle.com> wrote:
> This patch upgrades the sunvnet driver to support VIO protocol version 1.6.
> In particular, it adds per-port MTU negotiation, allowing MTUs other than
> ETH_FRAMELEN with ports using newer VIO protocol versions.
>
> Signed-off-by: David L Stevens <david.stevens@oracle.com>
> ---
> arch/sparc/include/asm/vio.h | 44 ++++++++++++++-
> arch/sparc/kernel/viohs.c | 14 ++++-
> drivers/net/ethernet/sun/sunvnet.c | 104 +++++++++++++++++++++++++++++------
> drivers/net/ethernet/sun/sunvnet.h | 3 +
> 4 files changed, 143 insertions(+), 22 deletions(-)
>
> diff --git a/arch/sparc/include/asm/vio.h b/arch/sparc/include/asm/vio.h
> index 5c0ebe7..107d4a4 100644
> --- a/arch/sparc/include/asm/vio.h
> +++ b/arch/sparc/include/asm/vio.h
> @@ -65,6 +65,7 @@ struct vio_dring_register {
> u16 options;
> #define VIO_TX_DRING 0x0001
> #define VIO_RX_DRING 0x0002
> +#define VIO_RX_DRING_DATA 0x0004
> u16 resv;
> u32 num_cookies;
> struct ldc_trans_cookie cookies[0];
> @@ -80,6 +81,8 @@ struct vio_dring_unregister {
> #define VIO_PKT_MODE 0x01 /* Packet based transfer */
> #define VIO_DESC_MODE 0x02 /* In-band descriptors */
> #define VIO_DRING_MODE 0x03 /* Descriptor rings */
> +/* in vers >= 1.2, VIO_DRING_MODE is 0x04 and transfer mode is a bitmask */
> +#define VIO_NEW_DRING_MODE 0x04
>
> struct vio_dring_data {
> struct vio_msg_tag tag;
> @@ -209,10 +212,20 @@ struct vio_net_attr_info {
> u8 addr_type;
> #define VNET_ADDR_ETHERMAC 0x01
> u16 ack_freq;
> - u32 resv1;
> + u8 plnk_updt;
> +#define PHYSLINK_UPDATE_NONE 0x00
> +#define PHYSLINK_UPDATE_STATE 0x01
> +#define PHYSLINK_UPDATE_STATE_ACK 0x02
> +#define PHYSLINK_UPDATE_STATE_NACK 0x03
> + u8 options;
> + u16 resv1;
> u64 addr;
> u64 mtu;
> - u64 resv2[3];
> + u16 cflags;
> +#define VNET_LSO_IPV4_CAPAB 0x0001
> + u16 ipv4_lso_maxlen;
> + u32 resv2;
> + u64 resv3[2];
> };
>
> #define VNET_NUM_MCAST 7
> @@ -370,6 +383,33 @@ struct vio_driver_state {
> struct vio_driver_ops *ops;
> };
>
> +static inline bool vio_version_before(struct vio_driver_state *vio,
> + u16 major, u16 minor)
> +{
> + u32 have = (u32)vio->ver.major << 16 | vio->ver.minor;
> + u32 want = (u32)major << 16 | minor;
> +
> + return have < want;
> +}
> +
> +static inline bool vio_version_after(struct vio_driver_state *vio,
> + u16 major, u16 minor)
> +{
> + u32 have = (u32)vio->ver.major << 16 | vio->ver.minor;
> + u32 want = (u32)major << 16 | minor;
> +
> + return have > want;
> +}
> +
> +static inline bool vio_version_after_eq(struct vio_driver_state *vio,
> + u16 major, u16 minor)
> +{
> + u32 have = (u32)vio->ver.major << 16 | vio->ver.minor;
> + u32 want = (u32)major << 16 | minor;
> +
> + return have >= want;
> +}
> +
> #define viodbg(TYPE, f, a...) \
> do { if (vio->debug & VIO_DEBUG_##TYPE) \
> printk(KERN_INFO "vio: ID[%lu] " f, \
> diff --git a/arch/sparc/kernel/viohs.c b/arch/sparc/kernel/viohs.c
> index f8e7dd5..7ef081a 100644
> --- a/arch/sparc/kernel/viohs.c
> +++ b/arch/sparc/kernel/viohs.c
> @@ -426,6 +426,13 @@ static int process_dreg_info(struct vio_driver_state *vio,
> if (vio->dr_state & VIO_DR_STATE_RXREG)
> goto send_nack;
>
> + /* v1.6 and higher, ACK with desired, supported mode, or NACK */
> + if (vio_version_after_eq(vio, 1, 6)) {
> + if (!(pkt->options & VIO_TX_DRING))
> + goto send_nack;
> + pkt->options = VIO_TX_DRING;
> + }
> +
> BUG_ON(vio->desc_buf);
>
> vio->desc_buf = kzalloc(pkt->descr_size, GFP_ATOMIC);
> @@ -453,8 +460,11 @@ static int process_dreg_info(struct vio_driver_state *vio,
> pkt->tag.stype = VIO_SUBTYPE_ACK;
> pkt->dring_ident = ++dr->ident;
>
> - viodbg(HS, "SEND DRING_REG ACK ident[%llx]\n",
> - (unsigned long long) pkt->dring_ident);
> + viodbg(HS, "SEND DRING_REG ACK ident[%llx] "
> + "ndesc[%u] dsz[%u] opt[0x%x] ncookies[%u]\n",
> + (unsigned long long) pkt->dring_ident,
> + pkt->num_descr, pkt->descr_size, pkt->options,
> + pkt->num_cookies);
>
> len = (sizeof(*pkt) +
> (dr->ncookies * sizeof(struct ldc_trans_cookie)));
> diff --git a/drivers/net/ethernet/sun/sunvnet.c b/drivers/net/ethernet/sun/sunvnet.c
> index 763cdfc..a9638c1 100644
> --- a/drivers/net/ethernet/sun/sunvnet.c
> +++ b/drivers/net/ethernet/sun/sunvnet.c
> @@ -15,6 +15,7 @@
> #include <linux/ethtool.h>
> #include <linux/etherdevice.h>
> #include <linux/mutex.h>
> +#include <linux/if_vlan.h>
>
> #include <asm/vio.h>
> #include <asm/ldc.h>
> @@ -41,6 +42,7 @@ static int __vnet_tx_trigger(struct vnet_port *port, u32 start);
>
> /* Ordered from largest major to lowest */
> static struct vio_version vnet_versions[] = {
> + { .major = 1, .minor = 6 },
> { .major = 1, .minor = 0 },
> };
>
> @@ -67,6 +69,7 @@ static int vnet_send_attr(struct vio_driver_state *vio)
> struct vnet_port *port = to_vnet_port(vio);
> struct net_device *dev = port->vp->dev;
> struct vio_net_attr_info pkt;
> + int framelen = ETH_FRAME_LEN;
> int i;
>
> memset(&pkt, 0, sizeof(pkt));
> @@ -74,19 +77,41 @@ static int vnet_send_attr(struct vio_driver_state *vio)
> pkt.tag.stype = VIO_SUBTYPE_INFO;
> pkt.tag.stype_env = VIO_ATTR_INFO;
> pkt.tag.sid = vio_send_sid(vio);
> - pkt.xfer_mode = VIO_DRING_MODE;
> + if (vio_version_before(vio, 1, 2))
> + pkt.xfer_mode = VIO_DRING_MODE;
> + else
> + pkt.xfer_mode = VIO_NEW_DRING_MODE;
> pkt.addr_type = VNET_ADDR_ETHERMAC;
> pkt.ack_freq = 0;
> for (i = 0; i < 6; i++)
> pkt.addr |= (u64)dev->dev_addr[i] << ((5 - i) * 8);
> - pkt.mtu = ETH_FRAME_LEN;
> + if (vio_version_after(vio, 1, 3)) {
> + if (port->rmtu) {
> + port->rmtu = min(VNET_MAXPACKET, port->rmtu);
> + pkt.mtu = port->rmtu;
> + } else {
> + port->rmtu = VNET_MAXPACKET;
> + pkt.mtu = port->rmtu;
> + }
> + if (vio_version_after_eq(vio, 1, 6))
> + pkt.options = VIO_TX_DRING;
> + } else if (vio_version_before(vio, 1, 3)) {
> + pkt.mtu = framelen;
> + } else { /* v1.3 */
> + pkt.mtu = framelen + VLAN_HLEN;
> + }
> +
> + pkt.plnk_updt = PHYSLINK_UPDATE_NONE;
> + pkt.cflags = 0;
>
> viodbg(HS, "SEND NET ATTR xmode[0x%x] atype[0x%x] addr[%llx] "
> - "ackfreq[%u] mtu[%llu]\n",
> + "ackfreq[%u] plnk_updt[0x%02x] opts[0x%02x] mtu[%llu] "
> + "cflags[0x%04x] lso_max[%u]\n",
> pkt.xfer_mode, pkt.addr_type,
> - (unsigned long long) pkt.addr,
> - pkt.ack_freq,
> - (unsigned long long) pkt.mtu);
> + (unsigned long long)pkt.addr,
> + pkt.ack_freq, pkt.plnk_updt, pkt.options,
> + (unsigned long long)pkt.mtu, pkt.cflags, pkt.ipv4_lso_maxlen);
> +
>
> return vio_ldc_send(vio, &pkt, sizeof(pkt));
> }
> @@ -94,18 +119,52 @@ static int vnet_send_attr(struct vio_driver_state *vio)
> static int handle_attr_info(struct vio_driver_state *vio,
> struct vio_net_attr_info *pkt)
> {
> - viodbg(HS, "GOT NET ATTR INFO xmode[0x%x] atype[0x%x] addr[%llx] "
> - "ackfreq[%u] mtu[%llu]\n",
> + struct vnet_port *port = to_vnet_port(vio);
> + u64 localmtu;
> + u8 xfer_mode;
> +
> + viodbg(HS, "GOT NET ATTR xmode[0x%x] atype[0x%x] addr[%llx] "
> + "ackfreq[%u] plnk_updt[0x%02x] opts[0x%02x] mtu[%llu] "
> + " (rmtu[%llu]) cflags[0x%04x] lso_max[%u]\n",
> pkt->xfer_mode, pkt->addr_type,
> - (unsigned long long) pkt->addr,
> - pkt->ack_freq,
> - (unsigned long long) pkt->mtu);
> + (unsigned long long)pkt->addr,
> + pkt->ack_freq, pkt->plnk_updt, pkt->options,
> + (unsigned long long)pkt->mtu, port->rmtu, pkt->cflags,
> + pkt->ipv4_lso_maxlen);
>
> pkt->tag.sid = vio_send_sid(vio);
>
> - if (pkt->xfer_mode != VIO_DRING_MODE ||
> + xfer_mode = pkt->xfer_mode;
> + /* for version < 1.2, VIO_DRING_MODE = 0x3 and no bitmask */
> + if (vio_version_before(vio, 1, 2) && xfer_mode == VIO_DRING_MODE)
> + xfer_mode = VIO_NEW_DRING_MODE;
> +
> + /* MTU negotiation:
> + * < v1.3 - ETH_FRAME_LEN exactly
> + * > v1.3 - MIN(pkt.mtu, VNET_MAXPACKET, port->rmtu) and change
> + * pkt->mtu for ACK
> + * = v1.3 - ETH_FRAME_LEN + VLAN_HLEN exactly
> + */
> + if (vio_version_before(vio, 1, 3)) {
> + localmtu = ETH_FRAME_LEN;
> + } else if (vio_version_after(vio, 1, 3)) {
> + localmtu = port->rmtu ? port->rmtu : VNET_MAXPACKET;
> + localmtu = min(pkt->mtu, localmtu);
> + pkt->mtu = localmtu;
> + } else { /* v1.3 */
> + localmtu = ETH_FRAME_LEN + VLAN_HLEN;
> + }
> + port->rmtu = localmtu;
> +
> + /* for version >= 1.6, ACK packet mode we support */
> + if (vio_version_after_eq(vio, 1, 6)) {
> + pkt->xfer_mode = VIO_NEW_DRING_MODE;
> + pkt->options = VIO_TX_DRING;
> + }
> +
> + if (!(xfer_mode | VIO_NEW_DRING_MODE) ||
> pkt->addr_type != VNET_ADDR_ETHERMAC ||
> - pkt->mtu != ETH_FRAME_LEN) {
> + pkt->mtu != localmtu) {
> viodbg(HS, "SEND NET ATTR NACK\n");
>
> pkt->tag.stype = VIO_SUBTYPE_NACK;
> @@ -114,7 +173,14 @@ static int handle_attr_info(struct vio_driver_state *vio,
>
> return -ECONNRESET;
> } else {
> - viodbg(HS, "SEND NET ATTR ACK\n");
> + viodbg(HS, "SEND NET ATTR ACK xmode[0x%x] atype[0x%x] "
> + "addr[%llx] ackfreq[%u] plnk_updt[0x%02x] opts[0x%02x] "
> + "mtu[%llu] (rmtu[%llu]) cflags[0x%04x] lso_max[%u]\n",
> + pkt->xfer_mode, pkt->addr_type,
> + (unsigned long long)pkt->addr,
> + pkt->ack_freq, pkt->plnk_updt, pkt->options,
> + (unsigned long long)pkt->mtu, port->rmtu, pkt->cflags,
> + pkt->ipv4_lso_maxlen);
>
> pkt->tag.stype = VIO_SUBTYPE_ACK;
>
> @@ -210,7 +276,7 @@ static int vnet_rx_one(struct vnet_port *port, unsigned int len,
> int err;
>
> err = -EMSGSIZE;
> - if (unlikely(len < ETH_ZLEN || len > ETH_FRAME_LEN)) {
> + if (unlikely(len < ETH_ZLEN || len > port->rmtu)) {
> dev->stats.rx_length_errors++;
> goto out_dropped;
> }
> @@ -555,8 +621,10 @@ static void vnet_event(void *arg, int event)
> vio_link_state_change(vio, event);
> spin_unlock_irqrestore(&vio->lock, flags);
>
> - if (event == LDC_EVENT_RESET)
> + if (event == LDC_EVENT_RESET) {
> + port->rmtu = 0;
> vio_port_up(vio);
> + }
> return;
> }
>
> @@ -1048,8 +1116,8 @@ static int vnet_port_alloc_tx_bufs(struct vnet_port *port)
> void *dring;
>
> for (i = 0; i < VNET_TX_RING_SIZE; i++) {
> - void *buf = kzalloc(ETH_FRAME_LEN + 8, GFP_KERNEL);
> - int map_len = (ETH_FRAME_LEN + 7) & ~7;
> + void *buf = kzalloc(VNET_MAXPACKET + 8, GFP_KERNEL);
This patch doesn't change the VNET_MAXPACKET to 64k, but the patch 2/3 changes
it to 64k+. Allocating buffers of size VNET_MAXPACKET always can consume too much
memory for every port/LDC, that would be more than 32MB. You may want to allocate
buffers based on the mtu that is negotiated, so that this memory used only when
such large packets are accepted by the peer.
-Raghuram
> + int map_len = (VNET_MAXPACKET + 7) & ~7;
>
> err = -ENOMEM;
> if (!buf)
> diff --git a/drivers/net/ethernet/sun/sunvnet.h b/drivers/net/ethernet/sun/sunvnet.h
> index da49337..986e04b 100644
> --- a/drivers/net/ethernet/sun/sunvnet.h
> +++ b/drivers/net/ethernet/sun/sunvnet.h
> @@ -11,6 +11,7 @@
> */
> #define VNET_TX_TIMEOUT (5 * HZ)
>
> +#define VNET_MAXPACKET 1518ULL /* ETH_FRAMELEN + VLAN_HDR */
> #define VNET_TX_RING_SIZE 512
> #define VNET_TX_WAKEUP_THRESH(dr) ((dr)->pending / 4)
>
> @@ -44,6 +45,8 @@ struct vnet_port {
> u32 stop_rx_idx;
> bool stop_rx;
> bool start_cons;
> +
> + u64 rmtu;
> };
>
> static inline struct vnet_port *to_vnet_port(struct vio_driver_state *vio)
> --
> 1.7.1
>
> --
> 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
* [PATCH] [trivial] wireless:rtlwifi: Fix typo in rtl wifi drivers
From: Masanari Iida @ 2014-09-18 3:29 UTC (permalink / raw)
To: trivial, linux-kernel, Larry.Finger, linux-wireless
Cc: netdev, davem, Masanari Iida
This patch fix spelling typo "sleeped" in printk, found in
multiple rtlwifi drivers.
Signed-off-by: Masanari Iida <standby24x7@gmail.com>
---
drivers/net/wireless/rtlwifi/rtl8188ee/phy.c | 2 +-
drivers/net/wireless/rtlwifi/rtl8192ce/phy.c | 2 +-
drivers/net/wireless/rtlwifi/rtl8192cu/phy.c | 2 +-
drivers/net/wireless/rtlwifi/rtl8192de/phy.c | 2 +-
drivers/net/wireless/rtlwifi/rtl8192se/phy.c | 2 +-
drivers/net/wireless/rtlwifi/rtl8723ae/phy.c | 2 +-
drivers/net/wireless/rtlwifi/rtl8723be/phy.c | 2 +-
7 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/net/wireless/rtlwifi/rtl8188ee/phy.c b/drivers/net/wireless/rtlwifi/rtl8188ee/phy.c
index 1cd6c16..3d689e8 100644
--- a/drivers/net/wireless/rtlwifi/rtl8188ee/phy.c
+++ b/drivers/net/wireless/rtlwifi/rtl8188ee/phy.c
@@ -2019,7 +2019,7 @@ static bool _rtl88ee_phy_set_rf_power_state(struct ieee80211_hw *hw,
RT_RF_OFF_LEVL_HALT_NIC);
} else {
RT_TRACE(rtlpriv, COMP_RF, DBG_DMESG,
- "Set ERFON sleeped:%d ms\n",
+ "Set ERFON slept:%d ms\n",
jiffies_to_msecs(jiffies - ppsc->
last_sleep_jiffies));
ppsc->last_awake_jiffies = jiffies;
diff --git a/drivers/net/wireless/rtlwifi/rtl8192ce/phy.c b/drivers/net/wireless/rtlwifi/rtl8192ce/phy.c
index 98b2230..ac3ffa2 100644
--- a/drivers/net/wireless/rtlwifi/rtl8192ce/phy.c
+++ b/drivers/net/wireless/rtlwifi/rtl8192ce/phy.c
@@ -480,7 +480,7 @@ static bool _rtl92ce_phy_set_rf_power_state(struct ieee80211_hw *hw,
RT_RF_OFF_LEVL_HALT_NIC);
} else {
RT_TRACE(rtlpriv, COMP_RF, DBG_DMESG,
- "Set ERFON sleeped:%d ms\n",
+ "Set ERFON slept:%d ms\n",
jiffies_to_msecs(jiffies -
ppsc->
last_sleep_jiffies));
diff --git a/drivers/net/wireless/rtlwifi/rtl8192cu/phy.c b/drivers/net/wireless/rtlwifi/rtl8192cu/phy.c
index 9831ff1..519d6e5 100644
--- a/drivers/net/wireless/rtlwifi/rtl8192cu/phy.c
+++ b/drivers/net/wireless/rtlwifi/rtl8192cu/phy.c
@@ -437,7 +437,7 @@ static bool _rtl92cu_phy_set_rf_power_state(struct ieee80211_hw *hw,
RT_RF_OFF_LEVL_HALT_NIC);
} else {
RT_TRACE(rtlpriv, COMP_RF, DBG_DMESG,
- "Set ERFON sleeped:%d ms\n",
+ "Set ERFON slept:%d ms\n",
jiffies_to_msecs(jiffies -
ppsc->last_sleep_jiffies));
ppsc->last_awake_jiffies = jiffies;
diff --git a/drivers/net/wireless/rtlwifi/rtl8192de/phy.c b/drivers/net/wireless/rtlwifi/rtl8192de/phy.c
index 592125a..7c02247 100644
--- a/drivers/net/wireless/rtlwifi/rtl8192de/phy.c
+++ b/drivers/net/wireless/rtlwifi/rtl8192de/phy.c
@@ -3121,7 +3121,7 @@ bool rtl92d_phy_set_rf_power_state(struct ieee80211_hw *hw,
RT_RF_OFF_LEVL_HALT_NIC);
} else {
RT_TRACE(rtlpriv, COMP_POWER, DBG_DMESG,
- "awake, sleeped:%d ms state_inap:%x\n",
+ "awake, slept:%d ms state_inap:%x\n",
jiffies_to_msecs(jiffies -
ppsc->last_sleep_jiffies),
rtlpriv->psc.state_inap);
diff --git a/drivers/net/wireless/rtlwifi/rtl8192se/phy.c b/drivers/net/wireless/rtlwifi/rtl8192se/phy.c
index 77c5b5f..6853ace 100644
--- a/drivers/net/wireless/rtlwifi/rtl8192se/phy.c
+++ b/drivers/net/wireless/rtlwifi/rtl8192se/phy.c
@@ -565,7 +565,7 @@ bool rtl92s_phy_set_rf_power_state(struct ieee80211_hw *hw,
RT_RF_OFF_LEVL_HALT_NIC);
} else {
RT_TRACE(rtlpriv, COMP_POWER, DBG_DMESG,
- "awake, sleeped:%d ms state_inap:%x\n",
+ "awake, slept:%d ms state_inap:%x\n",
jiffies_to_msecs(jiffies -
ppsc->
last_sleep_jiffies),
diff --git a/drivers/net/wireless/rtlwifi/rtl8723ae/phy.c b/drivers/net/wireless/rtlwifi/rtl8723ae/phy.c
index 3ea78af..95f9902 100644
--- a/drivers/net/wireless/rtlwifi/rtl8723ae/phy.c
+++ b/drivers/net/wireless/rtlwifi/rtl8723ae/phy.c
@@ -1486,7 +1486,7 @@ static bool _rtl8723ae_phy_set_rf_power_state(struct ieee80211_hw *hw,
RT_RF_OFF_LEVL_HALT_NIC);
} else {
RT_TRACE(rtlpriv, COMP_RF, DBG_DMESG,
- "Set ERFON sleeped:%d ms\n",
+ "Set ERFON slept:%d ms\n",
jiffies_to_msecs(jiffies -
ppsc->last_sleep_jiffies));
ppsc->last_awake_jiffies = jiffies;
diff --git a/drivers/net/wireless/rtlwifi/rtl8723be/phy.c b/drivers/net/wireless/rtlwifi/rtl8723be/phy.c
index 1575ef9..b49968e 100644
--- a/drivers/net/wireless/rtlwifi/rtl8723be/phy.c
+++ b/drivers/net/wireless/rtlwifi/rtl8723be/phy.c
@@ -2041,7 +2041,7 @@ static bool _rtl8723be_phy_set_rf_power_state(struct ieee80211_hw *hw,
RT_RF_OFF_LEVL_HALT_NIC);
} else {
RT_TRACE(rtlpriv, COMP_RF, DBG_DMESG,
- "Set ERFON sleeped:%d ms\n",
+ "Set ERFON slept:%d ms\n",
jiffies_to_msecs(jiffies -
ppsc->last_sleep_jiffies));
ppsc->last_awake_jiffies = jiffies;
--
2.1.0.238.gce1d3a9
^ permalink raw reply related
* RE: [PATCH v2 3/4] net: stmmac: add support for Intel Quark X1000 [RESEND TO MAILING LIST]
From: Kweh, Hock Leong @ 2014-09-18 3:18 UTC (permalink / raw)
To: 'David Miller'
Cc: 'peppe.cavallaro@st.com',
'rayagond@vayavyalabs.com',
'vbridgers2013@gmail.com',
'srinivas.kandagatla@st.com', 'wens@csie.org',
'netdev@vger.kernel.org',
'linux-kernel@vger.kernel.org', Ong, Boon Leong
> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Wednesday, September 17, 2014 12:56 PM
> From: "Kweh, Hock Leong" <hock.leong.kweh@intel.com>
> Date: Wed, 17 Sep 2014 02:41:39 +0000
>
> > Thanks for the pointer. I did a quickly checking on the class number
> > to see if I could use it for differentiation the ports number.
> > Whereas I found them both have the same class number as well. Below
> > shows the "lspci" dump to all the PCI devices on Quark X1000 Galileo
> > board (Ethernet controllers are
> > 00:14.6 and 00:14.7). Very unfortunately we are unlikely to use the
> > class number as well as pci_device_id for the differentiation.
> ...
> > 00:14.6 "Class 0200" "8086" "0937" "8086" "0937"
> > 00:14.7 "Class 0200" "8086" "0937" "8086" "0937"
>
> Are you kidding me? It's a perfect way to identify this device, it
> properly uses PCI_CLASS_NETWORK_ETHERNET (0x0200) in both cases and
> this will not match any other function on this PCI device at all.
>
> Please do as I suggested and use the PCI class for the differentiation
> and matching during probing.
Hi David,
Thanks for your feedback so far. Appreciate it.
Sorry to my poorly written description that may have caused some confusion. My sincere apology here.
Unfortunately, I don't really grasp your idea clearly based on your responses which I appreciate them a lot.
Sorry for the long description below but I hope to clearly pen down my thinking process so that you can follow my thinking incrementally without being confused.
So, let's roll back a bit so that with my following description, you can help correct me if my understanding of using PCI function ID to differentiate PHY port that is associated with each Ethernet controller is wrong:
The high-level idea about the change that I made for STMMAC IP inside Quark is as follow:
(1) Based on Quark-specific PCI ID declared inside stmmac_id_table[], the probe() function is
called to continue setting-up STMMAC for Quark.
@@ -228,11 +303,13 @@ static int stmmac_pci_resume(struct pci_dev *pdev)
#define STMMAC_VENDOR_ID 0x700
#define STMMAC_DEVICE_ID 0x1108
+#define STMMAC_QUARK_X1000_ID 0x0937
static const struct pci_device_id stmmac_id_table[] = {
{PCI_DEVICE(STMMAC_VENDOR_ID, STMMAC_DEVICE_ID), PCI_ANY_ID,
PCI_ANY_ID, CHIP_STMICRO},
{PCI_VDEVICE(STMICRO, PCI_DEVICE_ID_STMICRO_MAC), CHIP_STMICRO},
+ {PCI_VDEVICE(INTEL, STMMAC_QUARK_X1000_ID), CHIP_QUARK_X1000},
{}
};
(2) Back-ground on STMMAC hardware configuration on Intel Galileo Gen 1 & Gen 2 platforms:
Intel Quark SoC has 2 MAC controller as described by lspci output below:
00:14.6 Class 0200: 8086:0937 ====> 1st MAC controller
00:14.7 Class 0200: 8086:0937 ====> 2nd MAC controller
These Galileo boards use the same Intel Quark SoC and there is only one PHY connect to the 1st MAC [00:14.6 Class 0200: 8086:0937] The 2nd MAC [00:14.7 Class 0200: 8086:0937] is NOT connected to any PHY at all.
So, it appears to me that the only way that I can differentiate between 1st & 2nd MAC are based on PCI function ID, i.e. 14.6 & 14.7. Therefore, within the probe() function, for Intel Quark SoC only, the function performs next-level discovery of 1st or 2nd MAC controller through quark_run_time_config() function.
For other PCI ID (currently STMICRO_MAC) there is NO next-level discovery involved as rt_config is NULL.
Changes shown below:
static struct platform_data platform_info[] = { @@ -59,15 +65,76 @@ static struct platform_data platform_info[] = {
.phy_reset = NULL,
.phy_mask = 0,
.pbl = 32,
+ .fixed_burst = 0,
.burst_len = DMA_AXI_BLEN_256,
+ .rt_config = NULL, ===================> no 2nd-level discovery for other PCI ID
+ },
+ [CHIP_QUARK_X1000] = {
+ .phy_addr = 1,
+ .interface = PHY_INTERFACE_MODE_RMII,
+ .clk_csr = 2,
+ .has_gmac = 1,
+ .force_sf_dma_mode = 1,
+ .multicast_filter_bins = HASH_TABLE_SIZE,
+ .unicast_filter_entries = 1,
+ .phy_reset = NULL,
+ .phy_mask = 0,
+ .pbl = 16,
+ .fixed_burst = 1,
+ .burst_len = DMA_AXI_BLEN_256,
+ .rt_config = &quark_run_time_config, ==========> Quark specific 2nd-level discovery
+ },
+};
(3) Within quark_run_time_config(), due to the only way to differentiate 1st or 2nd MAC controller according to difference in function ID explained above, the following changes are made:
+static void quark_run_time_config(int chip_id, struct pci_dev *pdev) {
+ const char *board_name = dmi_get_system_info(DMI_BOARD_NAME);
+ int i;
+ int func_num = PCI_FUNC(pdev->devfn);
+
+ if (!board_name)
+ return;
+
+ for (i = 0; i < ARRAY_SIZE(quark_x1000_phy_info); i++) {
+ if ((!strcmp(quark_x1000_phy_info[i].board_name, board_name)) &&
+ quark_x1000_phy_info[i].pci_func_num == func_num)
+ platform_info[chip_id].phy_addr =
+ quark_x1000_phy_info[i].phy_address;
+ }
+}
The reasons for the above proposed condition checks, i.e. "board name" & "pci function name" are below:
a) As described above, the only difference in both instance of STMMAC IP inside Intel Quark SoC is the function ID,
so I have proposed to use function ID to be the decision point here to differentiate 1st MAC from 2nd MAC.
b) Allow future expansion of any other Intel Quark platforms with specific need to fix PHY address
c) A PHY address set as "-1" is to mark that the PHY (associated with function ID) is not connected to MAC, which
is being used here for the 2 Galileo boards -> 2nd MAC port not connected with PHY.
Finally, based on the above description, it appears to me that using PCI function ID to decode seems viable for Intel Quark specific hardware configuration.
Appreciate your time and any feedback is very much appreciated.
Thanks.
Regards,
Wilson
^ permalink raw reply
* [PATCH/RFC repost 8/8] hack: ofproto: enable odp select action
From: Simon Horman @ 2014-09-18 1:55 UTC (permalink / raw)
To: dev, netdev; +Cc: Pravin Shelar, Jesse Gross, Thomas Graf, Simon Horman
In-Reply-To: <1411005311-11752-1-git-send-email-simon.horman@netronome.com>
This is a quick hack to enable the datapath group select action.
It is in lieu of some combination of:
* probing
* run-time configuration by the end-use.
* run-time heuristic to use the action as appropriate
Signed-off-by: Simon Horman <simon.horman@netronome.com>
---
ofproto/ofproto-dpif-xlate.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index b08a821..eca617d 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -2896,7 +2896,7 @@ xlate_select_group_userspace(struct xlate_ctx *ctx, struct group_dpif *group)
static void
xlate_select_group(struct xlate_ctx *ctx, struct group_dpif *group)
{
- if (ctx->xbridge->dp_select_group) {
+ if (true /*ctx->xbridge->dp_select_group*/ ) {
xlate_select_group_datapath(ctx, group);
} else {
xlate_select_group_userspace(ctx, group);
--
2.0.1
^ permalink raw reply related
* [PATCH/RFC repost 7/8] ofproto: translate datapath select group action
From: Simon Horman @ 2014-09-18 1:55 UTC (permalink / raw)
To: dev, netdev; +Cc: Pravin Shelar, Jesse Gross, Thomas Graf, Simon Horman
In-Reply-To: <1411005311-11752-1-git-send-email-simon.horman@netronome.com>
This add support for the select group action to ovs-vswtichd.
This new feature is currently disabled in xlate_select_group()
because ctx->xbridge->dp_select_group is always false.
This patch is a prototype and has several limitations:
* It assumes that no actions follow a select group action
because the resulting packet after a select group action may
differ depending on the bucket used. It may be possible
to address this problem using recirculation. Or to not use
the datapath select group in such situations. In any case
this patch does not solve this problem or even prevent it
from occurring.
* If recirculation occurs inside more than one bucket then
it seems that separate recirculation ids would be required
for each occurrence. This patch does not solve this problem or even
prevent it from occurring.
* No attempt is made to handle per-bucket statistics.
This would most likely require further datapath modifications.
Signed-off-by: Simon Horman <simon.horman@netronome.com>
---
ofproto/ofproto-dpif-xlate.c | 108 ++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 107 insertions(+), 1 deletion(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 6e33a27..b08a821 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -103,6 +103,9 @@ struct xbridge {
* False if the datapath supports only 8-byte (or shorter) userdata. */
bool variable_length_userdata;
+ /* True if datapath supports select group action */
+ bool dp_select_group;
+
/* Number of MPLS label stack entries that the datapath supports
* in matches. */
size_t max_mpls_depth;
@@ -329,6 +332,9 @@ static void xlate_report(struct xlate_ctx *, const char *);
static void xlate_table_action(struct xlate_ctx *, ofp_port_t in_port,
uint8_t table_id, bool may_packet_in,
bool honor_table_miss);
+static void xlate_group_stats(struct xlate_ctx *, struct group_dpif *,
+ struct ofputil_bucket *);
+static void xlate_group_bucket(struct xlate_ctx *, struct ofputil_bucket *);
static bool input_vid_is_valid(uint16_t vid, struct xbundle *, bool warn);
static uint16_t input_vid_to_vlan(const struct xbundle *, uint16_t vid);
static void output_normal(struct xlate_ctx *, const struct xbundle *,
@@ -2353,6 +2359,61 @@ fix_sflow_action(struct xlate_ctx *ctx)
ctx->sflow_odp_port, ctx->sflow_n_outputs, cookie);
}
+/* Compose bucket for SELECT_GROUP action. */
+static void
+compose_bucket_action(struct xlate_ctx *ctx, struct group_dpif *group,
+ struct ofputil_bucket *bucket)
+{
+ size_t bucket_offset, actions_offset;
+ struct ofpbuf *odp_actions = ctx->xout->odp_actions;
+
+ bucket_offset = nl_msg_start_nested(odp_actions,
+ OVS_SELECT_GROUP_ATTR_BUCKET);
+
+ nl_msg_put_u16(odp_actions, OVS_BUCKET_ATTR_WEIGHT, bucket->weight);
+
+ actions_offset = nl_msg_start_nested(odp_actions, OVS_BUCKET_ATTR_ACTIONS);
+
+ xlate_group_bucket(ctx, bucket);
+ xlate_group_stats(ctx, group, bucket);
+
+ nl_msg_end_nested(odp_actions, actions_offset);
+ nl_msg_end_nested(odp_actions, bucket_offset);
+
+ ctx->xout->slow |= commit_odp_actions(&ctx->xin->flow, &ctx->base_flow,
+ odp_actions, &ctx->xout->wc);
+}
+
+/* Compose SELECT_GROUP action.
+ * May be called multiple times to add multiple buckets.
+ * The first call should pass SIZE_MAX as the value for offset_
+ * and subsequent calls should pass returned by the previous call.
+ * The last call should pass NULL as the bucket parameter
+ * and previous calls should pass a valid bucket to add to the
+ * select group. */
+static size_t
+compose_select_group_action(struct xlate_ctx *ctx, struct group_dpif *group,
+ struct ofputil_bucket *bucket,
+ const size_t offset_)
+{
+ size_t offset = offset_;
+
+ if (bucket != NULL) {
+ if (offset == SIZE_MAX) {
+ offset = nl_msg_start_nested(ctx->xout->odp_actions,
+ OVS_ACTION_ATTR_SELECT_GROUP);
+ }
+
+ compose_bucket_action(ctx, group, bucket);
+ } else {
+ if (offset != SIZE_MAX) {
+ nl_msg_end_nested(ctx->xout->odp_actions, offset);
+ }
+ }
+
+ return offset;
+}
+
static enum slow_path_reason
process_special(struct xlate_ctx *ctx, const struct flow *flow,
const struct xport *xport, const struct ofpbuf *packet)
@@ -2775,7 +2836,40 @@ xlate_ff_group(struct xlate_ctx *ctx, struct group_dpif *group)
}
static void
-xlate_select_group(struct xlate_ctx *ctx, struct group_dpif *group)
+xlate_select_group_datapath(struct xlate_ctx *ctx, struct group_dpif *group)
+{
+ struct flow_wildcards *wc = &ctx->xout->wc;
+ struct ofputil_bucket *bucket;
+ const struct list *buckets;
+ struct flow old_flow = ctx->xin->flow;
+ size_t offset = SIZE_MAX;
+
+ group_dpif_get_buckets(group, &buckets);
+ LIST_FOR_EACH (bucket, list_node, buckets) {
+ if (bucket_is_alive(ctx, bucket, 0)) {
+ offset = compose_select_group_action(ctx, group, bucket, offset);
+
+ /* Roll back flow to previous state.
+ *
+ * This allows composition of the actions for subsequent
+ * buckets in such a way that they apply to the state of the
+ * packet present before the select group action.
+ *
+ * XXX: Assumes no actions follow the select group action.
+ * Handle with recirculation?
+ */
+ ctx->xin->flow = old_flow;
+ }
+ }
+
+ if (offset != SIZE_MAX) {
+ memset(&wc->masks.dl_dst, 0xff, sizeof wc->masks.dl_dst);
+ compose_select_group_action(ctx, group, NULL, offset);
+ }
+}
+
+static void
+xlate_select_group_userspace(struct xlate_ctx *ctx, struct group_dpif *group)
{
struct flow_wildcards *wc = &ctx->xout->wc;
struct ofputil_bucket *bucket;
@@ -2800,6 +2894,16 @@ xlate_select_group(struct xlate_ctx *ctx, struct group_dpif *group)
}
static void
+xlate_select_group(struct xlate_ctx *ctx, struct group_dpif *group)
+{
+ if (ctx->xbridge->dp_select_group) {
+ xlate_select_group_datapath(ctx, group);
+ } else {
+ xlate_select_group_userspace(ctx, group);
+ }
+}
+
+static void
xlate_group_action__(struct xlate_ctx *ctx, struct group_dpif *group)
{
ctx->in_group = true;
@@ -2990,6 +3094,8 @@ compose_recirculate_action(struct xlate_ctx *ctx,
ofpacts_len = ofpacts_base_len -
((uint8_t *)ofpact_current - (uint8_t *)ofpacts_base);
+ /* XXX: If recirculation occurs inside buckets of a select group action
+ * then multiple recirculation ids may be required. */
if (ctx->rule) {
id = rule_dpif_get_recirc_id(ctx->rule);
} else {
--
2.0.1
^ permalink raw reply related
* [PATCH/RFC repost 6/8] datapath: validation of select group action
From: Simon Horman @ 2014-09-18 1:55 UTC (permalink / raw)
To: dev, netdev; +Cc: Pravin Shelar, Jesse Gross, Thomas Graf, Simon Horman
In-Reply-To: <1411005311-11752-1-git-send-email-simon.horman@netronome.com>
Allow validation and copying of select group actions.
This completes the prototype select group action implementation
in the datapath. Subsequent patches will add support to ovs-vswtichd.
Signed-off-by: Simon Horman <simon.horman@netronome.com>
---
datapath/flow_netlink.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 102 insertions(+)
diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c
index 6c74841..90eddba 100644
--- a/datapath/flow_netlink.c
+++ b/datapath/flow_netlink.c
@@ -1497,6 +1497,94 @@ static int validate_and_copy_sample(const struct nlattr *attr,
return 0;
}
+static int validate_and_copy_bucket(const struct nlattr *attr,
+ const struct sw_flow_key *key, int depth,
+ struct sw_flow_actions **sfa,
+ __be16 eth_type, __be16 vlan_tci)
+{
+ const struct nlattr *attrs[OVS_BUCKET_ATTR_MAX + 1];
+ const struct nlattr *weight, *actions;
+ const struct nlattr *a;
+ int rem, start, err, st_acts;
+
+ memset(attrs, 0, sizeof(attrs));
+ nla_for_each_nested(a, attr, rem) {
+ int type = nla_type(a);
+ if (!type || type > OVS_BUCKET_ATTR_MAX || attrs[type])
+ return -EINVAL;
+ attrs[type] = a;
+ }
+ if (rem)
+ return -EINVAL;
+
+ weight = attrs[OVS_BUCKET_ATTR_WEIGHT];
+ if (!weight || nla_len(weight) != sizeof(u16))
+ return -EINVAL;
+
+ actions = attrs[OVS_BUCKET_ATTR_ACTIONS];
+ if (!actions || (nla_len(actions) && nla_len(actions) < NLA_HDRLEN))
+ return -EINVAL;
+
+ /* validation done, copy sample action. */
+ start = add_nested_action_start(sfa, OVS_SELECT_GROUP_ATTR_BUCKET);
+ if (start < 0)
+ return start;
+ err = add_action(sfa, OVS_BUCKET_ATTR_WEIGHT,
+ nla_data(weight), sizeof(u16));
+ if (err)
+ return err;
+ st_acts = add_nested_action_start(sfa, OVS_SAMPLE_ATTR_ACTIONS);
+ if (st_acts < 0)
+ return st_acts;
+
+ err = __ovs_nla_copy_actions(actions, key, depth + 1, sfa,
+ eth_type, vlan_tci);
+ if (err)
+ return err;
+
+ add_nested_action_end(*sfa, st_acts);
+ add_nested_action_end(*sfa, start);
+
+ return 0;
+}
+
+static int validate_and_copy_select_group(const struct nlattr *attr,
+ const struct sw_flow_key *key,
+ int depth,
+ struct sw_flow_actions **sfa,
+ __be16 eth_type, __be16 vlan_tci)
+{
+ bool have_bucket = false;
+ const struct nlattr *a;
+ int rem, start, err;
+
+ start = add_nested_action_start(sfa, OVS_ACTION_ATTR_SELECT_GROUP);
+ if (start < 0)
+ return start;
+
+ nla_for_each_nested(a, attr, rem) {
+ int type = nla_type(a);
+
+ if (!type || type > OVS_SAMPLE_ATTR_MAX)
+ return -EINVAL;
+
+ /* Only possible type is OVS_SELECT_GROUP_ATTR_BUCKET */
+ if ((nla_len(a) && nla_len(a) < NLA_HDRLEN))
+ return -EINVAL;
+ err = validate_and_copy_bucket(a, key, depth, sfa,
+ eth_type, vlan_tci);
+ if (err < 0)
+ return err;
+ have_bucket = true;
+ }
+ if (rem || !have_bucket)
+ return -EINVAL;
+
+ add_nested_action_end(*sfa, start);
+
+ return 0;
+}
+
static int validate_tp_port(const struct sw_flow_key *flow_key,
__be16 eth_type)
{
@@ -1750,6 +1838,7 @@ static int __ovs_nla_copy_actions(const struct nlattr *attr,
[OVS_ACTION_ATTR_POP_VLAN] = 0,
[OVS_ACTION_ATTR_SET] = (u32)-1,
[OVS_ACTION_ATTR_SAMPLE] = (u32)-1,
+ [OVS_ACTION_ATTR_SELECT_GROUP] = (u32)-1,
[OVS_ACTION_ATTR_HASH] = sizeof(struct ovs_action_hash)
};
const struct ovs_action_push_vlan *vlan;
@@ -1856,6 +1945,19 @@ static int __ovs_nla_copy_actions(const struct nlattr *attr,
skip_copy = true;
break;
+ case OVS_ACTION_ATTR_SELECT_GROUP:
+ /* Nothing may come after a select group */
+ if (!last_action(a, rem))
+ return -EINVAL;
+
+ err = validate_and_copy_select_group(a, key, depth,
+ sfa, eth_type,
+ vlan_tci);
+ if (err)
+ return err;
+ skip_copy = true;
+ break;
+
default:
return -EINVAL;
}
--
2.0.1
^ permalink raw reply related
* [PATCH/RFC repost 5/8] datapath: Move last_action() helper to datapath.h
From: Simon Horman @ 2014-09-18 1:55 UTC (permalink / raw)
To: dev, netdev; +Cc: Pravin Shelar, Jesse Gross, Thomas Graf, Simon Horman
In-Reply-To: <1411005311-11752-1-git-send-email-simon.horman@netronome.com>
This is in preparation for using last_action() from
more than one C file as part of supporting an odp select group action.
Signed-off-by: Simon Horman <simon.horman@netronome.com>
---
datapath/actions.c | 6 ------
datapath/datapath.h | 5 +++++
2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/datapath/actions.c b/datapath/actions.c
index 51ca40b..9d27234 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -752,11 +752,6 @@ static int output_userspace(struct datapath *dp, struct sk_buff *skb,
return ovs_dp_upcall(dp, skb, &upcall);
}
-static bool last_action(const struct nlattr *a, int rem)
-{
- return a->nla_len == rem;
-}
-
static int sample(struct datapath *dp, struct sk_buff *skb,
const struct nlattr *attr)
{
@@ -841,7 +836,6 @@ static int select_group(struct datapath *dp, struct sk_buff *skb,
const struct nlattr *best_bucket = NULL;
const struct nlattr *acts_list;
const struct nlattr *bucket;
- struct sk_buff *sample_skb;
u32 best_score = 0;
u32 basis;
u32 i = 0;
diff --git a/datapath/datapath.h b/datapath/datapath.h
index c5d3c86..74a15e6 100644
--- a/datapath/datapath.h
+++ b/datapath/datapath.h
@@ -209,4 +209,9 @@ do { \
if (net_ratelimit()) \
pr_info("netlink: " fmt, ##__VA_ARGS__); \
} while (0)
+
+static inline bool last_action(const struct nlattr *a, int rem)
+{
+ return a->nla_len == rem;
+}
#endif /* datapath.h */
--
2.0.1
^ permalink raw reply related
* [PATCH/RFC repost 4/8] datapath: execution of select group action
From: Simon Horman @ 2014-09-18 1:55 UTC (permalink / raw)
To: dev, netdev; +Cc: Pravin Shelar, Jesse Gross, Thomas Graf, Simon Horman
In-Reply-To: <1411005311-11752-1-git-send-email-simon.horman@netronome.com>
Allow execution of select group action in the datapath.
A subsequent patch will add validation and copying of
the select group action in the datapath.
The selection algorithm used is based on the RSS hash.
This was chosen because it resembles the algorithm currently
used by the implementation of select groups in ovs-vswitchd.
It may well be that in this case it is more efficient to handle
things in ovs-vswitchd, avoiding the cost of hashing on each packet.
Or that the hashing mechanism used can be optimised somehow. However,
we would like to avoid focusing on these questions of this particular
implementation.
The purpose of this patch is to form part of a prototype for a select
group action. The current Open Flow specification allows for the
implementation to select an algorithm. And we have made a separate
proposal to allow the selection algorithm to be configured via Open Flow.
Thus the algorithm and used and its implementation are not central
to the prototype.
Signed-off-by: Simon Horman <simon.horman@netronome.com>
---
datapath/actions.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 70 insertions(+)
diff --git a/datapath/actions.c b/datapath/actions.c
index 8d18848..51ca40b 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -809,6 +809,72 @@ static int sample(struct datapath *dp, struct sk_buff *skb,
return 0;
}
+const struct nlattr *bucket_actions(const struct nlattr *attr)
+{
+ const struct nlattr *a;
+ int rem;
+
+ for (a = nla_data(attr), rem = nla_len(attr); rem > 0;
+ a = nla_next(a, &rem)) {
+ if (nla_type(a) == OVS_BUCKET_ATTR_ACTIONS) {
+ return a;
+ }
+ }
+
+ return NULL;
+}
+
+static u16 bucket_weight(const struct nlattr *attr)
+{
+ const struct nlattr *weight;
+
+ /* validate_and_copy_bucket() ensures that the first
+ * attribute is OVS_BUCKET_ATTR_WEIGHT */
+ weight = nla_data(attr);
+ BUG_ON(nla_type(weight) != OVS_BUCKET_ATTR_WEIGHT);
+ return nla_get_u16(weight);
+}
+
+static int select_group(struct datapath *dp, struct sk_buff *skb,
+ const struct nlattr *attr)
+{
+ const struct nlattr *best_bucket = NULL;
+ const struct nlattr *acts_list;
+ const struct nlattr *bucket;
+ struct sk_buff *sample_skb;
+ u32 best_score = 0;
+ u32 basis;
+ u32 i = 0;
+ int rem;
+
+ basis = skb_get_hash(skb);
+
+ /* Only possible type of attributes is OVS_SELECT_GROUP_ATTR_BUCKET */
+ for (bucket = nla_data(attr), rem = nla_len(attr); rem > 0;
+ bucket = nla_next(bucket, &rem)) {
+ uint16_t weight = bucket_weight(bucket);
+ // XXX: This hashing seems expensive
+ u32 score = (jhash_1word(i, basis) & 0xffff) * weight;
+
+ if (score >= best_score) {
+ best_bucket = bucket;
+ best_score = score;
+ }
+ i++;
+ }
+
+ acts_list = bucket_actions(best_bucket);
+
+ /* A select group action is always the final action so
+ * there is no need to clone the skb in case of side effects.
+ * Instead just take a reference to it which will be released
+ * by do_execute_actions(). */
+ skb_get(skb);
+
+ return do_execute_actions(dp, skb, nla_data(acts_list),
+ nla_len(acts_list));
+}
+
static void execute_hash(struct sk_buff *skb, const struct nlattr *attr)
{
struct sw_flow_key *key = OVS_CB(skb)->pkt_key;
@@ -986,6 +1052,10 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
case OVS_ACTION_ATTR_SAMPLE:
err = sample(dp, skb, a);
break;
+
+ case OVS_ACTION_ATTR_SELECT_GROUP:
+ err = select_group(dp, skb, a);
+ break;
}
if (unlikely(err)) {
--
2.0.1
^ permalink raw reply related
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