* Re: [PATCH] netback: don't store invalid vif pointer
From: David Miller @ 2014-12-09 23:30 UTC (permalink / raw)
To: JBeulich; +Cc: wei.liu2, Ian.Campbell, xen-devel, netdev
In-Reply-To: <5486EF48020000780004E1B8@mail.emea.novell.com>
From: "Jan Beulich" <JBeulich@suse.com>
Date: Tue, 09 Dec 2014 11:47:04 +0000
> When xenvif_alloc() fails, it returns a non-NULL error indicator. To
> avoid eventual races, we shouldn't store that into struct backend_info
> as readers of it only check for NULL.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next 0/9] r8169:update hardware ephy parameter
From: David Miller @ 2014-12-09 23:35 UTC (permalink / raw)
To: hau; +Cc: netdev, nic_swsd, linux-kernel
In-Reply-To: <1418143563-7652-1-git-send-email-hau@realtek.com>
From: Chunhao Lin <hau@realtek.com>
Date: Wed, 10 Dec 2014 00:45:54 +0800
> Update hardware ephy parameter to improve pcie compatibility.
This really doesn't tell me anything, I really dislike patch
series like this one.
All of the programming is magic values to magic offsets.
You aren't even trying to describe in the commit log message exactly
what kind of settings are being changed, and exactly how those changes
achieve the stated goal.
Furthermore, the commit description makes no sense at all to me.
How can programming the ethernet MAC PHY have any influence on PCI-E
bus compatability? Or are you programming the PCI bus interface's
PHY?
In what way are you adjusting which settings and in what way do those
adjustments help improve PCI-E bus behavior?
You absolutely must describe exactly what the new programming
is actually doing, precisely, and in detail. I want to know
if some kind of timings are being adjusted, and in what way.
Are some fifo limits being changes? If so, in what way, and
why does that help.
You have to describe what you are doing. Short and non-informative
commit log messages alongside random changes to undocumented magic
constant registers is simply unacceptable.
^ permalink raw reply
* Re: [PATCH net-next 0/9] r8169:update hardware ephy parameter
From: Francois Romieu @ 2014-12-09 23:47 UTC (permalink / raw)
To: Chunhao Lin; +Cc: netdev, nic_swsd, linux-kernel
In-Reply-To: <1418143563-7652-1-git-send-email-hau@realtek.com>
Chunhao Lin <hau@realtek.com> :
> Update hardware ephy parameter to improve pcie compatibility.
Neither the code nor the commit message helps to figure which
kind of behavioral change should/could be expected.
Bandwidth ? Latency ? Power management ? Compliance ? Stability ?
--
Ueimor
^ permalink raw reply
* Re: [PATCH net-next 5/9] r8169:update rtl8168fb ephy parameter
From: Francois Romieu @ 2014-12-09 23:48 UTC (permalink / raw)
To: Chunhao Lin; +Cc: netdev, nic_swsd, linux-kernel
In-Reply-To: <1418143563-7652-6-git-send-email-hau@realtek.com>
Chunhao Lin <hau@realtek.com> :
[rtl_hw_start_8168f_1 and rtl_hw_start_8168f_2 changes]
Different ephy_info data, same code. You may consider factoring it out.
--
Ueimor
^ permalink raw reply
* Re: [PATCH net-next 7/9] r8169:update rtl8168dp ephy parameter
From: Francois Romieu @ 2014-12-09 23:50 UTC (permalink / raw)
To: Chunhao Lin; +Cc: netdev, nic_swsd, linux-kernel
In-Reply-To: <1418143563-7652-8-git-send-email-hau@realtek.com>
Chunhao Lin <hau@realtek.com> :
[...]
> diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
> index a979519..42eda35 100644
> --- a/drivers/net/ethernet/realtek/r8169.c
> +++ b/drivers/net/ethernet/realtek/r8169.c
[...]
> static void rtl_hw_start_8168d_4(struct rtl8169_private *tp)
> {
> void __iomem *ioaddr = tp->mmio_addr;
> struct pci_dev *pdev = tp->pci_dev;
> static const struct ephy_info e_info_8168d_4[] = {
> - { 0x0b, ~0, 0x48 },
> - { 0x19, 0x20, 0x50 },
> - { 0x0c, ~0, 0x20 }
> + { 0x0b, 0x0000, 0x0048 },
> + { 0x19, 0x0020, 0x0050 },
> + { 0x0c, 0x0100, 0x0020 },
> + { 0x10, 0x0004, 0x0000 }
> };
> - int i;
>
> rtl_csi_access_enable_1(tp);
>
> - rtl_tx_performance_tweak(pdev, 0x5 << MAX_READ_REQUEST_SHIFT);
> + if (tp->dev->mtu <= ETH_DATA_LEN)
> + rtl_tx_performance_tweak(pdev, 0x5 << MAX_READ_REQUEST_SHIFT);
[...]
> @@ -6328,11 +6308,8 @@ static void rtl_hw_start_8168(struct net_device *dev)
> break;
>
> case RTL_GIGA_MAC_VER_28:
> - rtl_hw_start_8168d_4(tp);
> - break;
> -
> case RTL_GIGA_MAC_VER_31:
> - rtl_hw_start_8168dp(tp);
> + rtl_hw_start_8168d_4(tp);
RTL_GIGA_MAC_VER_28 would thus use a mtu dependant rtl_tx_performance_tweak
in its hw_start handler but would not include one in its jumbo_ops helpers.
It does not seem completely right.
--
Ueimor
^ permalink raw reply
* [PATCH] net: openvswitch: Support masked set actions.
From: Jarno Rajahalme @ 2014-12-10 0:10 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA, dev-yBygre7rU0TnMu66kgdUjQ
OVS userspace already probes the openvswitch kernel module for
OVS_ACTION_ATTR_SET_MASKED support. This patch adds the kernel module
implementation of masked set actions.
The existing set action sets many fields at once. When only a subset
of the IP header fields, for example, should be modified, all the IP
fields need to be exact matched so that the other field values can be
copied to the set action. A masked set action allows modification of
an arbitrary subset of the supported header bits without requiring the
rest to be matched.
Masked set action is now supported for all writeable key types, except
for the tunnel key. The set tunnel action is an exception as any
input tunnel info is cleared before action processing starts, so there
is no tunnel info to mask.
The kernel module converts all (non-tunnel) set actions to masked set
actions. This makes action processing more uniform, and results in
less branching and duplicating the action processing code. When
returning actions to userspace, the fully masked set actions are
converted back to normal set actions. We use a kernel internal action
code to be able to tell the userspace provided and converted masked
set actions apart.
Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
---
include/uapi/linux/openvswitch.h | 22 ++-
net/openvswitch/actions.c | 367 ++++++++++++++++++++++++--------------
net/openvswitch/flow_netlink.c | 168 ++++++++++++++---
3 files changed, 396 insertions(+), 161 deletions(-)
diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
index 3a6dcaa..f4bd703 100644
--- a/include/uapi/linux/openvswitch.h
+++ b/include/uapi/linux/openvswitch.h
@@ -564,6 +564,12 @@ struct ovs_action_hash {
* @OVS_ACTION_ATTR_SET: Replaces the contents of an existing header. The
* single nested %OVS_KEY_ATTR_* attribute specifies a header to modify and its
* value.
+ * @OVS_ACTION_ATTR_SET_MASKED: Replaces the contents of an existing header. A
+ * nested %OVS_KEY_ATTR_* attribute specifies a header to modify, its value,
+ * and a mask. For every bit set in the mask, the corresponding bit value
+ * is copied from the value to the packet header field, rest of the bits are
+ * left unchanged. The non-masked value bits must be passed in as zeroes.
+ * Masking is not supported for the %OVS_KEY_ATTR_TUNNEL attribute.
* @OVS_ACTION_ATTR_PUSH_VLAN: Push a new outermost 802.1Q header onto the
* packet.
* @OVS_ACTION_ATTR_POP_VLAN: Pop the outermost 802.1Q header off the packet.
@@ -582,6 +588,9 @@ struct ovs_action_hash {
* Only a single header can be set with a single %OVS_ACTION_ATTR_SET. Not all
* fields within a header are modifiable, e.g. the IPv4 protocol and fragment
* type may not be changed.
+ *
+ * @OVS_ACTION_ATTR_SET_TO_MASKED: Kernel internal masked set action translated
+ * from the @OVS_ACTION_ATTR_SET.
*/
enum ovs_action_attr {
@@ -596,8 +605,19 @@ enum ovs_action_attr {
OVS_ACTION_ATTR_HASH, /* struct ovs_action_hash. */
OVS_ACTION_ATTR_PUSH_MPLS, /* struct ovs_action_push_mpls. */
OVS_ACTION_ATTR_POP_MPLS, /* __be16 ethertype. */
+ OVS_ACTION_ATTR_SET_MASKED, /* One nested OVS_KEY_ATTR_* including
+ * data immediately followed by a mask.
+ * The data must be zero for the unmasked
+ * bits. */
+
+ __OVS_ACTION_ATTR_MAX, /* Nothing past this will be accepted
+ * from userspace. */
- __OVS_ACTION_ATTR_MAX
+#ifdef __KERNEL__
+ OVS_ACTION_ATTR_SET_TO_MASKED, /* Kernel module internal masked
+ * set action converted from
+ * OVS_ACTION_ATTR_SET. */
+#endif
};
#define OVS_ACTION_ATTR_MAX (__OVS_ACTION_ATTR_MAX - 1)
diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index 764fdc3..ee57b9b 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -184,10 +184,15 @@ static int pop_mpls(struct sk_buff *skb, struct sw_flow_key *key,
return 0;
}
-static int set_mpls(struct sk_buff *skb, struct sw_flow_key *key,
- const __be32 *mpls_lse)
+/* 'KEY' must not have any bits set outside of the 'MASK' */
+#define MASKED(OLD, KEY, MASK) ((KEY) | ((OLD) & ~(MASK)))
+#define SET_MASKED(OLD, KEY, MASK) (OLD) = MASKED(OLD, KEY, MASK)
+
+static int set_mpls(struct sk_buff *skb, struct sw_flow_key *flow_key,
+ const __be32 *mpls_lse, const __be32 *mask)
{
__be32 *stack;
+ __be32 lse;
int err;
err = skb_ensure_writable(skb, skb->mac_len + MPLS_HLEN);
@@ -195,14 +200,15 @@ static int set_mpls(struct sk_buff *skb, struct sw_flow_key *key,
return err;
stack = (__be32 *)skb_mpls_header(skb);
+ lse = MASKED(*stack, *mpls_lse, *mask);
if (skb->ip_summed == CHECKSUM_COMPLETE) {
- __be32 diff[] = { ~(*stack), *mpls_lse };
+ __be32 diff[] = { ~(*stack), lse };
skb->csum = ~csum_partial((char *)diff, sizeof(diff),
~skb->csum);
}
- *stack = *mpls_lse;
- key->mpls.top_lse = *mpls_lse;
+ *stack = lse;
+ flow_key->mpls.top_lse = lse;
return 0;
}
@@ -229,8 +235,21 @@ static int push_vlan(struct sk_buff *skb, struct sw_flow_key *key,
ntohs(vlan->vlan_tci) & ~VLAN_TAG_PRESENT);
}
-static int set_eth_addr(struct sk_buff *skb, struct sw_flow_key *key,
- const struct ovs_key_ethernet *eth_key)
+/* 'src' is already properly masked. */
+static void ether_addr_copy_masked(u8 *dst_, const u8 *src_, const u8 *mask_)
+{
+ u16 *dst = (u16 *)dst_;
+ const u16 *src = (const u16 *)src_;
+ const u16 *mask = (const u16 *)mask_;
+
+ SET_MASKED(dst[0], src[0], mask[0]);
+ SET_MASKED(dst[1], src[1], mask[1]);
+ SET_MASKED(dst[2], src[2], mask[2]);
+}
+
+static int set_eth_addr(struct sk_buff *skb, struct sw_flow_key *flow_key,
+ const struct ovs_key_ethernet *key,
+ const struct ovs_key_ethernet *mask)
{
int err;
err = skb_ensure_writable(skb, ETH_HLEN);
@@ -239,13 +258,15 @@ static int set_eth_addr(struct sk_buff *skb, struct sw_flow_key *key,
skb_postpull_rcsum(skb, eth_hdr(skb), ETH_ALEN * 2);
- ether_addr_copy(eth_hdr(skb)->h_source, eth_key->eth_src);
- ether_addr_copy(eth_hdr(skb)->h_dest, eth_key->eth_dst);
+ ether_addr_copy_masked(eth_hdr(skb)->h_source, key->eth_src,
+ mask->eth_src);
+ ether_addr_copy_masked(eth_hdr(skb)->h_dest, key->eth_dst,
+ mask->eth_dst);
ovs_skb_postpush_rcsum(skb, eth_hdr(skb), ETH_ALEN * 2);
- ether_addr_copy(key->eth.src, eth_key->eth_src);
- ether_addr_copy(key->eth.dst, eth_key->eth_dst);
+ ether_addr_copy(flow_key->eth.src, eth_hdr(skb)->h_source);
+ ether_addr_copy(flow_key->eth.dst, eth_hdr(skb)->h_dest);
return 0;
}
@@ -303,6 +324,15 @@ static void update_ipv6_checksum(struct sk_buff *skb, u8 l4_proto,
}
}
+static void mask_ipv6_addr(const __be32 old[4], const __be32 addr[4],
+ const __be32 mask[4], __be32 masked[4])
+{
+ masked[0] = MASKED(old[0], addr[0], mask[0]);
+ masked[1] = MASKED(old[1], addr[1], mask[1]);
+ masked[2] = MASKED(old[2], addr[2], mask[2]);
+ masked[3] = MASKED(old[3], addr[3], mask[3]);
+}
+
static void set_ipv6_addr(struct sk_buff *skb, u8 l4_proto,
__be32 addr[4], const __be32 new_addr[4],
bool recalculate_csum)
@@ -314,29 +344,29 @@ static void set_ipv6_addr(struct sk_buff *skb, u8 l4_proto,
memcpy(addr, new_addr, sizeof(__be32[4]));
}
-static void set_ipv6_tc(struct ipv6hdr *nh, u8 tc)
+static void set_ipv6_fl(struct ipv6hdr *nh, u32 fl, u32 mask)
{
- nh->priority = tc >> 4;
- nh->flow_lbl[0] = (nh->flow_lbl[0] & 0x0F) | ((tc & 0x0F) << 4);
+ /* Bits 21-24 are always unmasked, so this retains their values. */
+ SET_MASKED(nh->flow_lbl[0], (u8)(fl >> 16), (u8)(mask >> 16));
+ SET_MASKED(nh->flow_lbl[1], (u8)(fl >> 8), (u8)(mask >> 8));
+ SET_MASKED(nh->flow_lbl[2], (u8)fl, (u8)mask);
}
-static void set_ipv6_fl(struct ipv6hdr *nh, u32 fl)
+static void set_ip_ttl(struct sk_buff *skb, struct iphdr *nh, u8 new_ttl,
+ u8 mask)
{
- nh->flow_lbl[0] = (nh->flow_lbl[0] & 0xF0) | (fl & 0x000F0000) >> 16;
- nh->flow_lbl[1] = (fl & 0x0000FF00) >> 8;
- nh->flow_lbl[2] = fl & 0x000000FF;
-}
+ new_ttl = MASKED(nh->ttl, new_ttl, mask);
-static void set_ip_ttl(struct sk_buff *skb, struct iphdr *nh, u8 new_ttl)
-{
csum_replace2(&nh->check, htons(nh->ttl << 8), htons(new_ttl << 8));
nh->ttl = new_ttl;
}
-static int set_ipv4(struct sk_buff *skb, struct sw_flow_key *key,
- const struct ovs_key_ipv4 *ipv4_key)
+static int set_ipv4(struct sk_buff *skb, struct sw_flow_key *flow_key,
+ const struct ovs_key_ipv4 *key,
+ const struct ovs_key_ipv4 *mask)
{
struct iphdr *nh;
+ __be32 new_addr;
int err;
err = skb_ensure_writable(skb, skb_network_offset(skb) +
@@ -346,36 +376,48 @@ static int set_ipv4(struct sk_buff *skb, struct sw_flow_key *key,
nh = ip_hdr(skb);
- if (ipv4_key->ipv4_src != nh->saddr) {
- set_ip_addr(skb, nh, &nh->saddr, ipv4_key->ipv4_src);
- key->ipv4.addr.src = ipv4_key->ipv4_src;
- }
+ /* Setting an IP addresses is typically only a side effect of
+ * matching on them in the current userspace implementation, so it
+ * makes sense to check if the value actually changed. */
+ if (mask->ipv4_src) {
+ new_addr = MASKED(nh->saddr, key->ipv4_src, mask->ipv4_src);
- if (ipv4_key->ipv4_dst != nh->daddr) {
- set_ip_addr(skb, nh, &nh->daddr, ipv4_key->ipv4_dst);
- key->ipv4.addr.dst = ipv4_key->ipv4_dst;
+ if (unlikely(new_addr != nh->saddr)) {
+ set_ip_addr(skb, nh, &nh->saddr, new_addr);
+ flow_key->ipv4.addr.src = new_addr;
+ }
}
+ if (mask->ipv4_dst) {
+ new_addr = MASKED(nh->daddr, key->ipv4_dst, mask->ipv4_dst);
- if (ipv4_key->ipv4_tos != nh->tos) {
- ipv4_change_dsfield(nh, 0, ipv4_key->ipv4_tos);
- key->ip.tos = nh->tos;
+ if (unlikely(new_addr != nh->daddr)) {
+ set_ip_addr(skb, nh, &nh->daddr, new_addr);
+ flow_key->ipv4.addr.dst = new_addr;
+ }
}
-
- if (ipv4_key->ipv4_ttl != nh->ttl) {
- set_ip_ttl(skb, nh, ipv4_key->ipv4_ttl);
- key->ip.ttl = ipv4_key->ipv4_ttl;
+ if (mask->ipv4_tos) {
+ ipv4_change_dsfield(nh, ~mask->ipv4_tos, key->ipv4_tos);
+ flow_key->ip.tos = nh->tos;
+ }
+ if (mask->ipv4_ttl) {
+ set_ip_ttl(skb, nh, key->ipv4_ttl, mask->ipv4_ttl);
+ flow_key->ip.ttl = nh->ttl;
}
return 0;
}
-static int set_ipv6(struct sk_buff *skb, struct sw_flow_key *key,
- const struct ovs_key_ipv6 *ipv6_key)
+static bool is_ipv6_mask_nonzero(const __be32 addr[4])
+{
+ return !!(addr[0] | addr[1] | addr[2] | addr[3]);
+}
+
+static int set_ipv6(struct sk_buff *skb, struct sw_flow_key *flow_key,
+ const struct ovs_key_ipv6 *key,
+ const struct ovs_key_ipv6 *mask)
{
struct ipv6hdr *nh;
int err;
- __be32 *saddr;
- __be32 *daddr;
err = skb_ensure_writable(skb, skb_network_offset(skb) +
sizeof(struct ipv6hdr));
@@ -383,40 +425,59 @@ static int set_ipv6(struct sk_buff *skb, struct sw_flow_key *key,
return err;
nh = ipv6_hdr(skb);
- saddr = (__be32 *)&nh->saddr;
- daddr = (__be32 *)&nh->daddr;
-
- if (memcmp(ipv6_key->ipv6_src, saddr, sizeof(ipv6_key->ipv6_src))) {
- set_ipv6_addr(skb, ipv6_key->ipv6_proto, saddr,
- ipv6_key->ipv6_src, true);
- memcpy(&key->ipv6.addr.src, ipv6_key->ipv6_src,
- sizeof(ipv6_key->ipv6_src));
- }
- if (memcmp(ipv6_key->ipv6_dst, daddr, sizeof(ipv6_key->ipv6_dst))) {
+ /* Setting an IP addresses is typically only a side effect of
+ * matching on them in the current userspace implementation, so it
+ * makes sense to check if the value actually changed. */
+ if (is_ipv6_mask_nonzero(mask->ipv6_src)) {
+ __be32 *saddr = (__be32 *)&nh->saddr;
+ __be32 masked[4];
+
+ mask_ipv6_addr(saddr, key->ipv6_src, mask->ipv6_src, masked);
+
+ if (unlikely(memcmp(saddr, masked, sizeof masked))) {
+ set_ipv6_addr(skb, key->ipv6_proto, saddr, masked,
+ true);
+ memcpy(&flow_key->ipv6.addr.src, masked,
+ sizeof(flow_key->ipv6.addr.src));
+ }
+ }
+ if (is_ipv6_mask_nonzero(mask->ipv6_dst)) {
unsigned int offset = 0;
int flags = IP6_FH_F_SKIP_RH;
bool recalc_csum = true;
-
- if (ipv6_ext_hdr(nh->nexthdr))
- recalc_csum = ipv6_find_hdr(skb, &offset,
- NEXTHDR_ROUTING, NULL,
- &flags) != NEXTHDR_ROUTING;
-
- set_ipv6_addr(skb, ipv6_key->ipv6_proto, daddr,
- ipv6_key->ipv6_dst, recalc_csum);
- memcpy(&key->ipv6.addr.dst, ipv6_key->ipv6_dst,
- sizeof(ipv6_key->ipv6_dst));
+ __be32 *daddr = (__be32 *)&nh->daddr;
+ __be32 masked[4];
+
+ mask_ipv6_addr(daddr, key->ipv6_dst, mask->ipv6_dst, masked);
+
+ if (unlikely(memcmp(daddr, masked, sizeof masked))) {
+ if (ipv6_ext_hdr(nh->nexthdr))
+ recalc_csum = (ipv6_find_hdr(skb, &offset,
+ NEXTHDR_ROUTING,
+ NULL, &flags)
+ != NEXTHDR_ROUTING);
+
+ set_ipv6_addr(skb, key->ipv6_proto, daddr, masked,
+ recalc_csum);
+ memcpy(&flow_key->ipv6.addr.dst, masked,
+ sizeof(flow_key->ipv6.addr.dst));
+ }
+ }
+ if (mask->ipv6_tclass) {
+ ipv6_change_dsfield(nh, ~mask->ipv6_tclass, key->ipv6_tclass);
+ flow_key->ip.tos = ipv6_get_dsfield(nh);
+ }
+ if (mask->ipv6_label) {
+ set_ipv6_fl(nh, ntohl(key->ipv6_label),
+ ntohl(mask->ipv6_label));
+ flow_key->ipv6.label =
+ *(__be32 *)nh & htonl(IPV6_FLOWINFO_FLOWLABEL);
+ }
+ if (mask->ipv6_hlimit) {
+ SET_MASKED(nh->hop_limit, key->ipv6_hlimit, mask->ipv6_hlimit);
+ flow_key->ip.ttl = nh->hop_limit;
}
-
- set_ipv6_tc(nh, ipv6_key->ipv6_tclass);
- key->ip.tos = ipv6_get_dsfield(nh);
-
- set_ipv6_fl(nh, ntohl(ipv6_key->ipv6_label));
- key->ipv6.label = *(__be32 *)nh & htonl(IPV6_FLOWINFO_FLOWLABEL);
-
- nh->hop_limit = ipv6_key->ipv6_hlimit;
- key->ip.ttl = ipv6_key->ipv6_hlimit;
return 0;
}
@@ -426,28 +487,14 @@ static void set_tp_port(struct sk_buff *skb, __be16 *port,
{
inet_proto_csum_replace2(check, skb, *port, new_port, 0);
*port = new_port;
- skb_clear_hash(skb);
-}
-
-static void set_udp_port(struct sk_buff *skb, __be16 *port, __be16 new_port)
-{
- struct udphdr *uh = udp_hdr(skb);
-
- if (uh->check && skb->ip_summed != CHECKSUM_PARTIAL) {
- set_tp_port(skb, port, new_port, &uh->check);
-
- if (!uh->check)
- uh->check = CSUM_MANGLED_0;
- } else {
- *port = new_port;
- skb_clear_hash(skb);
- }
}
-static int set_udp(struct sk_buff *skb, struct sw_flow_key *key,
- const struct ovs_key_udp *udp_port_key)
+static int set_udp(struct sk_buff *skb, struct sw_flow_key *flow_key,
+ const struct ovs_key_udp *key,
+ const struct ovs_key_udp *mask)
{
struct udphdr *uh;
+ __be16 src, dst;
int err;
err = skb_ensure_writable(skb, skb_transport_offset(skb) +
@@ -456,23 +503,40 @@ static int set_udp(struct sk_buff *skb, struct sw_flow_key *key,
return err;
uh = udp_hdr(skb);
- if (udp_port_key->udp_src != uh->source) {
- set_udp_port(skb, &uh->source, udp_port_key->udp_src);
- key->tp.src = udp_port_key->udp_src;
- }
+ /* Either of the masks is non-zero, so do not bother checking them. */
+ src = MASKED(uh->source, key->udp_src, mask->udp_src);
+ dst = MASKED(uh->dest, key->udp_dst, mask->udp_dst);
- if (udp_port_key->udp_dst != uh->dest) {
- set_udp_port(skb, &uh->dest, udp_port_key->udp_dst);
- key->tp.dst = udp_port_key->udp_dst;
+ if (uh->check && skb->ip_summed != CHECKSUM_PARTIAL) {
+ if (likely(src != uh->source)) {
+ set_tp_port(skb, &uh->source, src, &uh->check);
+ flow_key->tp.src = src;
+ }
+ if (likely(dst != uh->dest)) {
+ set_tp_port(skb, &uh->dest, dst, &uh->check);
+ flow_key->tp.dst = dst;
+ }
+
+ if (unlikely(!uh->check))
+ uh->check = CSUM_MANGLED_0;
+ } else {
+ uh->source = src;
+ uh->dest = dst;
+ flow_key->tp.src = src;
+ flow_key->tp.dst = dst;
}
+ skb_clear_hash(skb);
+
return 0;
}
-static int set_tcp(struct sk_buff *skb, struct sw_flow_key *key,
- const struct ovs_key_tcp *tcp_port_key)
+static int set_tcp(struct sk_buff *skb, struct sw_flow_key *flow_key,
+ const struct ovs_key_tcp *key,
+ const struct ovs_key_tcp *mask)
{
struct tcphdr *th;
+ __be16 src, dst;
int err;
err = skb_ensure_writable(skb, skb_transport_offset(skb) +
@@ -481,50 +545,49 @@ static int set_tcp(struct sk_buff *skb, struct sw_flow_key *key,
return err;
th = tcp_hdr(skb);
- if (tcp_port_key->tcp_src != th->source) {
- set_tp_port(skb, &th->source, tcp_port_key->tcp_src, &th->check);
- key->tp.src = tcp_port_key->tcp_src;
+ src = MASKED(th->source, key->tcp_src, mask->tcp_src);
+ if (likely(src != th->source)) {
+ set_tp_port(skb, &th->source, src, &th->check);
+ flow_key->tp.src = src;
}
-
- if (tcp_port_key->tcp_dst != th->dest) {
- set_tp_port(skb, &th->dest, tcp_port_key->tcp_dst, &th->check);
- key->tp.dst = tcp_port_key->tcp_dst;
+ dst = MASKED(th->dest, key->tcp_dst, mask->tcp_dst);
+ if (likely(dst != th->dest)) {
+ set_tp_port(skb, &th->dest, dst, &th->check);
+ flow_key->tp.dst = dst;
}
+ skb_clear_hash(skb);
return 0;
}
-static int set_sctp(struct sk_buff *skb, struct sw_flow_key *key,
- const struct ovs_key_sctp *sctp_port_key)
+static int set_sctp(struct sk_buff *skb, struct sw_flow_key *flow_key,
+ const struct ovs_key_sctp *key,
+ const struct ovs_key_sctp *mask)
{
+ unsigned int sctphoff = skb_transport_offset(skb);
struct sctphdr *sh;
+ __le32 old_correct_csum, new_csum, old_csum;
int err;
- unsigned int sctphoff = skb_transport_offset(skb);
err = skb_ensure_writable(skb, sctphoff + sizeof(struct sctphdr));
if (unlikely(err))
return err;
sh = sctp_hdr(skb);
- if (sctp_port_key->sctp_src != sh->source ||
- sctp_port_key->sctp_dst != sh->dest) {
- __le32 old_correct_csum, new_csum, old_csum;
-
- old_csum = sh->checksum;
- old_correct_csum = sctp_compute_cksum(skb, sctphoff);
+ old_csum = sh->checksum;
+ old_correct_csum = sctp_compute_cksum(skb, sctphoff);
- sh->source = sctp_port_key->sctp_src;
- sh->dest = sctp_port_key->sctp_dst;
+ sh->source = MASKED(sh->source, key->sctp_src, mask->sctp_src);
+ sh->dest = MASKED(sh->dest, key->sctp_dst, mask->sctp_dst);
- new_csum = sctp_compute_cksum(skb, sctphoff);
+ new_csum = sctp_compute_cksum(skb, sctphoff);
- /* Carry any checksum errors through. */
- sh->checksum = old_csum ^ old_correct_csum ^ new_csum;
+ /* Carry any checksum errors through. */
+ sh->checksum = old_csum ^ old_correct_csum ^ new_csum;
- skb_clear_hash(skb);
- key->tp.src = sctp_port_key->sctp_src;
- key->tp.dst = sctp_port_key->sctp_dst;
- }
+ skb_clear_hash(skb);
+ flow_key->tp.src = sh->source;
+ flow_key->tp.dst = sh->dest;
return 0;
}
@@ -652,52 +715,77 @@ static void execute_hash(struct sk_buff *skb, struct sw_flow_key *key,
key->ovs_flow_hash = hash;
}
-static int execute_set_action(struct sk_buff *skb, struct sw_flow_key *key,
- const struct nlattr *nested_attr)
+static int execute_set_action(struct sk_buff *skb,
+ struct sw_flow_key *flow_key,
+ const struct nlattr *a)
+{
+ /* Only tunnel set execution is supported without a mask. */
+ if (nla_type(a) == OVS_KEY_ATTR_TUNNEL_INFO) {
+ OVS_CB(skb)->egress_tun_info = nla_data(a);
+ return 0;
+ }
+
+ return -EINVAL;
+
+}
+
+/* Mask is at the midpoint of the data. */
+#define get_mask(a, type) ((const type *)nla_data(a) + 1)
+
+static int execute_masked_set_action(struct sk_buff *skb,
+ struct sw_flow_key *flow_key,
+ const struct nlattr *a)
{
int err = 0;
- switch (nla_type(nested_attr)) {
+ switch (nla_type(a)) {
case OVS_KEY_ATTR_PRIORITY:
- skb->priority = nla_get_u32(nested_attr);
- key->phy.priority = skb->priority;
+ SET_MASKED(skb->priority, nla_get_u32(a), *get_mask(a, u32));
+ flow_key->phy.priority = skb->priority;
break;
case OVS_KEY_ATTR_SKB_MARK:
- skb->mark = nla_get_u32(nested_attr);
- key->phy.skb_mark = skb->mark;
+ SET_MASKED(skb->mark, nla_get_u32(a), *get_mask(a, u32));
+ flow_key->phy.skb_mark = skb->mark;
break;
case OVS_KEY_ATTR_TUNNEL_INFO:
- OVS_CB(skb)->egress_tun_info = nla_data(nested_attr);
+ /* Masked data not supported for tunnel. */
+ err = -EINVAL;
break;
case OVS_KEY_ATTR_ETHERNET:
- err = set_eth_addr(skb, key, nla_data(nested_attr));
+ err = set_eth_addr(skb, flow_key, nla_data(a),
+ get_mask(a, struct ovs_key_ethernet));
break;
case OVS_KEY_ATTR_IPV4:
- err = set_ipv4(skb, key, nla_data(nested_attr));
+ err = set_ipv4(skb, flow_key, nla_data(a),
+ get_mask(a, struct ovs_key_ipv4));
break;
case OVS_KEY_ATTR_IPV6:
- err = set_ipv6(skb, key, nla_data(nested_attr));
+ err = set_ipv6(skb, flow_key, nla_data(a),
+ get_mask(a, struct ovs_key_ipv6));
break;
case OVS_KEY_ATTR_TCP:
- err = set_tcp(skb, key, nla_data(nested_attr));
+ err = set_tcp(skb, flow_key, nla_data(a),
+ get_mask(a, struct ovs_key_tcp));
break;
case OVS_KEY_ATTR_UDP:
- err = set_udp(skb, key, nla_data(nested_attr));
+ err = set_udp(skb, flow_key, nla_data(a),
+ get_mask(a, struct ovs_key_udp));
break;
case OVS_KEY_ATTR_SCTP:
- err = set_sctp(skb, key, nla_data(nested_attr));
+ err = set_sctp(skb, flow_key, nla_data(a),
+ get_mask(a, struct ovs_key_sctp));
break;
case OVS_KEY_ATTR_MPLS:
- err = set_mpls(skb, key, nla_data(nested_attr));
+ err = set_mpls(skb, flow_key, nla_data(a), get_mask(a, __be32));
break;
}
@@ -817,6 +905,11 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
err = execute_set_action(skb, key, nla_data(a));
break;
+ case OVS_ACTION_ATTR_SET_MASKED:
+ case OVS_ACTION_ATTR_SET_TO_MASKED:
+ err = execute_masked_set_action(skb, key, nla_data(a));
+ break;
+
case OVS_ACTION_ATTR_SAMPLE:
err = sample(dp, skb, key, a);
break;
diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
index df3c7f2..276bb60 100644
--- a/net/openvswitch/flow_netlink.c
+++ b/net/openvswitch/flow_netlink.c
@@ -1609,23 +1609,44 @@ static int validate_and_copy_set_tun(const struct nlattr *attr,
return err;
}
+/* Return false if there are any non-masked bits set.
+ * Mask follows data immediately, before any netlink padding. */
+static bool validate_masked(u8 *data, int len)
+{
+ u8 *mask = data + len;
+
+ while (len--)
+ if (*data++ & ~*mask++)
+ return false;
+
+ return true;
+}
+
static int validate_set(const struct nlattr *a,
const struct sw_flow_key *flow_key,
struct sw_flow_actions **sfa,
- bool *set_tun, __be16 eth_type, bool log)
+ bool *skip_copy, __be16 eth_type, bool masked, bool log)
{
const struct nlattr *ovs_key = nla_data(a);
int key_type = nla_type(ovs_key);
+ size_t key_len;
/* There can be only one key in a action */
if (nla_total_size(nla_len(ovs_key)) != nla_len(a))
return -EINVAL;
+ key_len = nla_len(ovs_key);
+ if (masked)
+ key_len /= 2;
+
if (key_type > OVS_KEY_ATTR_MAX ||
- (ovs_key_lens[key_type] != nla_len(ovs_key) &&
+ (ovs_key_lens[key_type] != key_len &&
ovs_key_lens[key_type] != -1))
return -EINVAL;
+ if (masked && !validate_masked(nla_data(ovs_key), key_len))
+ return -EINVAL;
+
switch (key_type) {
const struct ovs_key_ipv4 *ipv4_key;
const struct ovs_key_ipv6 *ipv6_key;
@@ -1640,7 +1661,10 @@ static int validate_set(const struct nlattr *a,
if (eth_p_mpls(eth_type))
return -EINVAL;
- *set_tun = true;
+ if (masked)
+ return -EINVAL; /* Masked tunnel set not supported. */
+
+ *skip_copy = true;
err = validate_and_copy_set_tun(a, sfa, log);
if (err)
return err;
@@ -1650,32 +1674,46 @@ static int validate_set(const struct nlattr *a,
if (eth_type != htons(ETH_P_IP))
return -EINVAL;
- if (!flow_key->ip.proto)
- return -EINVAL;
-
ipv4_key = nla_data(ovs_key);
- if (ipv4_key->ipv4_proto != flow_key->ip.proto)
- return -EINVAL;
- if (ipv4_key->ipv4_frag != flow_key->ip.frag)
- return -EINVAL;
+ if (masked) {
+ const struct ovs_key_ipv4 *mask = ipv4_key + 1;
+ /* Non-writeable fields. */
+ if (mask->ipv4_proto || mask->ipv4_frag)
+ return -EINVAL;
+ } else {
+ if (ipv4_key->ipv4_proto != flow_key->ip.proto)
+ return -EINVAL;
+
+ if (ipv4_key->ipv4_frag != flow_key->ip.frag)
+ return -EINVAL;
+ }
break;
case OVS_KEY_ATTR_IPV6:
if (eth_type != htons(ETH_P_IPV6))
return -EINVAL;
- if (!flow_key->ip.proto)
- return -EINVAL;
-
ipv6_key = nla_data(ovs_key);
- if (ipv6_key->ipv6_proto != flow_key->ip.proto)
- return -EINVAL;
- if (ipv6_key->ipv6_frag != flow_key->ip.frag)
- return -EINVAL;
+ if (masked) {
+ const struct ovs_key_ipv6 *mask = ipv6_key + 1;
+ /* Non-writeable fields. */
+ if (mask->ipv6_proto || mask->ipv6_frag)
+ return -EINVAL;
+
+ /* Invalid bits in the flow label mask? */
+ if (ntohl(mask->ipv6_label) & 0xFFF00000)
+ return -EINVAL;
+ } else {
+ if (ipv6_key->ipv6_proto != flow_key->ip.proto)
+ return -EINVAL;
+
+ if (ipv6_key->ipv6_frag != flow_key->ip.frag)
+ return -EINVAL;
+ }
if (ntohl(ipv6_key->ipv6_label) & 0xFFF00000)
return -EINVAL;
@@ -1685,13 +1723,19 @@ static int validate_set(const struct nlattr *a,
if (flow_key->ip.proto != IPPROTO_TCP)
return -EINVAL;
- return validate_tp_port(flow_key, eth_type);
+ err = validate_tp_port(flow_key, eth_type);
+ if (err)
+ return err;
+ break;
case OVS_KEY_ATTR_UDP:
if (flow_key->ip.proto != IPPROTO_UDP)
return -EINVAL;
- return validate_tp_port(flow_key, eth_type);
+ err = validate_tp_port(flow_key, eth_type);
+ if (err)
+ return err;
+ break;
case OVS_KEY_ATTR_MPLS:
if (!eth_p_mpls(eth_type))
@@ -1702,12 +1746,43 @@ static int validate_set(const struct nlattr *a,
if (flow_key->ip.proto != IPPROTO_SCTP)
return -EINVAL;
- return validate_tp_port(flow_key, eth_type);
+ err = validate_tp_port(flow_key, eth_type);
+ if (err)
+ return err;
+ break;
default:
return -EINVAL;
}
+ /* Convert non-masked non-tunnel set actions to masked set actions. */
+ if (!masked && key_type != OVS_KEY_ATTR_TUNNEL) {
+ int start, len = key_len * 2;
+ struct nlattr *at;
+
+ *skip_copy = true;
+
+ start = add_nested_action_start(sfa,
+ OVS_ACTION_ATTR_SET_TO_MASKED,
+ log);
+ if (start < 0)
+ return start;
+
+ at = __add_action(sfa, key_type, NULL, len, log);
+ if (IS_ERR(at))
+ return PTR_ERR(at);
+
+ memcpy(nla_data(at), nla_data(ovs_key), key_len); /* Key. */
+ memset(nla_data(at) + key_len, 0xff, key_len); /* Mask. */
+ /* Clear non-writeable bits from otherwise writeable fields. */
+ if (key_type == OVS_KEY_ATTR_IPV6) {
+ struct ovs_key_ipv6 *mask = nla_data(at) + key_len;
+
+ mask->ipv6_label &= htonl(0x000FFFFF);
+ }
+ add_nested_action_end(*sfa, start);
+ }
+
return 0;
}
@@ -1770,6 +1845,7 @@ static int __ovs_nla_copy_actions(const struct nlattr *attr,
[OVS_ACTION_ATTR_PUSH_VLAN] = sizeof(struct ovs_action_push_vlan),
[OVS_ACTION_ATTR_POP_VLAN] = 0,
[OVS_ACTION_ATTR_SET] = (u32)-1,
+ [OVS_ACTION_ATTR_SET_MASKED] = (u32)-1,
[OVS_ACTION_ATTR_SAMPLE] = (u32)-1,
[OVS_ACTION_ATTR_HASH] = sizeof(struct ovs_action_hash)
};
@@ -1872,12 +1948,20 @@ static int __ovs_nla_copy_actions(const struct nlattr *attr,
break;
case OVS_ACTION_ATTR_SET:
- err = validate_set(a, key, sfa,
- &out_tnl_port, eth_type, log);
+ err = validate_set(a, key, sfa, &skip_copy, eth_type,
+ false, log);
if (err)
return err;
- skip_copy = out_tnl_port;
+ if (nla_type(nla_data(a)) == OVS_KEY_ATTR_TUNNEL)
+ out_tnl_port = true;
+ break;
+
+ case OVS_ACTION_ATTR_SET_MASKED:
+ err = validate_set(a, key, sfa, &skip_copy, eth_type,
+ true, log);
+ if (err)
+ return err;
break;
case OVS_ACTION_ATTR_SAMPLE:
@@ -1905,6 +1989,7 @@ static int __ovs_nla_copy_actions(const struct nlattr *attr,
return 0;
}
+/* 'key' must be the masked key. */
int ovs_nla_copy_actions(const struct nlattr *attr,
const struct sw_flow_key *key,
struct sw_flow_actions **sfa, bool log)
@@ -1992,6 +2077,31 @@ static int set_action_to_attr(const struct nlattr *a, struct sk_buff *skb)
return 0;
}
+static int masked_set_action_to_attr(const struct nlattr *a,
+ struct sk_buff *skb)
+{
+ const struct nlattr *ovs_key = nla_data(a);
+
+ if (nla_put(skb, OVS_ACTION_ATTR_SET_MASKED, nla_len(a), ovs_key))
+ return -EMSGSIZE;
+
+ return 0;
+}
+
+static int masked_set_action_to_set_action_attr(const struct nlattr *a,
+ struct sk_buff *skb)
+{
+ const struct nlattr *ovs_key = nla_data(a);
+ size_t key_len = nla_len(ovs_key) / 2;
+
+ /* Revert the conversion we did from a non-masked set action to
+ * masked set action. */
+ if (nla_put(skb, OVS_ACTION_ATTR_SET, nla_len(a) - key_len, ovs_key))
+ return -EMSGSIZE;
+
+ return 0;
+}
+
int ovs_nla_put_actions(const struct nlattr *attr, int len, struct sk_buff *skb)
{
const struct nlattr *a;
@@ -2007,6 +2117,18 @@ int ovs_nla_put_actions(const struct nlattr *attr, int len, struct sk_buff *skb)
return err;
break;
+ case OVS_ACTION_ATTR_SET_MASKED:
+ err = masked_set_action_to_attr(a, skb);
+ if (err)
+ return err;
+ break;
+
+ case OVS_ACTION_ATTR_SET_TO_MASKED:
+ err = masked_set_action_to_set_action_attr(a, skb);
+ if (err)
+ return err;
+ break;
+
case OVS_ACTION_ATTR_SAMPLE:
err = sample_action_to_attr(a, skb);
if (err)
--
1.7.10.4
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
^ permalink raw reply related
* Re: [PATCHv11 net-next 2/2] openvswitch: Add support for unique flow IDs.
From: Joe Stringer @ 2014-12-10 0:25 UTC (permalink / raw)
To: Pravin Shelar; +Cc: dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org, netdev, LKML
In-Reply-To: <CALnjE+o4tbQVTyVkzdOG8zj=qxPZ8Dv3vGHbYkBmghxsLgKkkw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 9 December 2014 at 10:32, Pravin Shelar <pshelar@nicira.com> wrote:
> On Tue, Dec 2, 2014 at 6:56 PM, Joe Stringer <joestringer@nicira.com> wrote:
>> @@ -662,11 +664,18 @@ static void get_dp_stats(const struct datapath *dp, struct ovs_dp_stats *stats,
>> }
>> }
>>
>> -static size_t ovs_flow_cmd_msg_size(const struct sw_flow_actions *acts)
>> +static size_t ovs_flow_cmd_msg_size(const struct sw_flow_actions *acts,
>> + const struct sw_flow_id *sfid)
>> {
>> + size_t sfid_len = 0;
>> +
>> + if (sfid && sfid->ufid_len)
>> + sfid_len = nla_total_size(sfid->ufid_len);
>> +
> Can you also use ufid_flags to determine exact msg size?
Yeah, that should be straightforward.
>> @@ -849,13 +876,30 @@ static int ovs_flow_cmd_new(struct sk_buff *skb, struct genl_info *info)
>> }
>>
>> /* Extract key. */
>> - ovs_match_init(&match, &new_flow->unmasked_key, &mask);
>> + ovs_match_init(&match, &key, &mask);
>> error = ovs_nla_get_match(&match, a[OVS_FLOW_ATTR_KEY],
>> a[OVS_FLOW_ATTR_MASK], log);
>> if (error)
>> goto err_kfree_flow;
>>
>> - ovs_flow_mask_key(&new_flow->key, &new_flow->unmasked_key, &mask);
>> + ovs_flow_mask_key(&new_flow->key, &key, &mask);
>> +
>> + /* Extract flow id. */
>> + error = ovs_nla_copy_ufid(a[OVS_FLOW_ATTR_UFID], &new_flow->ufid, log);
>> + if (error)
>> + goto err_kfree_flow;
>
> Returning zero in case of no UFID is strange. Can you check for UFID
> attribute and then copy ufid unconditionally?
Right, along with the changes I'll make to integrating unmasked_key
and UFID this should collapse into a single call to
ovs_nla_copy_identifier() which will handle both cases.
>> @@ -877,8 +921,12 @@ static int ovs_flow_cmd_new(struct sk_buff *skb, struct genl_info *info)
>> error = -ENODEV;
>> goto err_unlock_ovs;
>> }
>> +
>> /* Check if this is a duplicate flow */
>> - flow = ovs_flow_tbl_lookup(&dp->table, &new_flow->unmasked_key);
>> + if (new_flow->ufid)
>> + flow = ovs_flow_tbl_lookup_ufid(&dp->table, new_flow->ufid);
>> + if (!flow)
>> + flow = ovs_flow_tbl_lookup(&dp->table, &new_flow->key);
>
> Need tight checking, for example what if flow with UFID does not exist
> in UFID table but it exist in flow-table? This can complicate
> flow-update case where we can not assume UFID of new and old flow is
> same.
OK, I overlooked this for the UFID case. The non-UFID case does an
exact lookup later in the function, we could add a check in the UFID
case to compare the masked keys and return an error if they are
unequal.
>> @@ -980,45 +1032,34 @@ static int ovs_flow_cmd_set(struct sk_buff *skb, struct genl_info *info)
>> struct nlattr **a = info->attrs;
>> struct ovs_header *ovs_header = info->userhdr;
>> struct sw_flow_key key;
>> - struct sw_flow *flow;
>> + struct sw_flow *flow = NULL;
>> struct sw_flow_mask mask;
>> struct sk_buff *reply = NULL;
>> struct datapath *dp;
>> struct sw_flow_actions *old_acts = NULL, *acts = NULL;
>> struct sw_flow_match match;
>> + struct sw_flow_id *ufid;
>> + u32 ufid_flags = ovs_nla_get_ufid_flags(a[OVS_FLOW_ATTR_UFID_FLAGS]);
>> int error;
>> bool log = !a[OVS_FLOW_ATTR_PROBE];
>>
>> - /* Extract key. */
>> - error = -EINVAL;
>> - if (!a[OVS_FLOW_ATTR_KEY]) {
>> + /* Extract identifier. Take a copy to avoid "Wframe-larger-than=1024"
>> + * warning.
>> + */
> What if we limit the implementation of UFID to max 128 bit, does it
> still gives you the warning?
It will if we still use struct sw_flow_id, when we combine
UFID+unmasked key. Possibly not with just a 128bit array. I'll look
into it.
>> + error = ovs_nla_copy_ufid(a[OVS_FLOW_ATTR_UFID], &ufid, log);
>> + if (error)
>> + return error;
>> + if (a[OVS_FLOW_ATTR_KEY]) {
>> + ovs_match_init(&match, &key, &mask);
>> + error = ovs_nla_get_match(&match, a[OVS_FLOW_ATTR_KEY],
>> + a[OVS_FLOW_ATTR_MASK], log);
>> + } else if (!ufid) {
>> OVS_NLERR(log, "Flow key attribute not present in set flow.");
>> - goto error;
>> + error = -EINVAL;
>> }
>> -
>> - ovs_match_init(&match, &key, &mask);
>> - error = ovs_nla_get_match(&match, a[OVS_FLOW_ATTR_KEY],
>> - a[OVS_FLOW_ATTR_MASK], log);
>> if (error)
>> goto error;
>>
>> - /* Validate actions. */
>> - if (a[OVS_FLOW_ATTR_ACTIONS]) {
>> - acts = get_flow_actions(a[OVS_FLOW_ATTR_ACTIONS], &key, &mask,
>> - log);
>> - if (IS_ERR(acts)) {
>> - error = PTR_ERR(acts);
>> - goto error;
>> - }
>> -
>> - /* Can allocate before locking if have acts. */
>> - reply = ovs_flow_cmd_alloc_info(acts, info, false);
>> - if (IS_ERR(reply)) {
>> - error = PTR_ERR(reply);
>> - goto err_kfree_acts;
>> - }
>> - }
>> -
> Why are you moving this action validation under ovs-lock?
The thought was that flow_cmd_set may receive UFID and not key/mask.
One could imagine a command that sends a UFID and actions, telling OVS
kmod to update just the actions. Masked key is required for
ovs_nla_copy_actions(), so in this case a lookup would be required to
get a masked key.
Perhaps the better alternative for the moment is to still require flow
key and mask for this command (just as we do for flow_cmd_new). That
would simplify this change greatly, and doesn't affect current OVS
userspace.
>> @@ -1194,9 +1254,18 @@ unlock:
>>
>> static int ovs_flow_cmd_dump(struct sk_buff *skb, struct netlink_callback *cb)
>> {
>> + struct nlattr *a[__OVS_FLOW_ATTR_MAX];
>> struct ovs_header *ovs_header = genlmsg_data(nlmsg_data(cb->nlh));
>> struct table_instance *ti;
>> struct datapath *dp;
>> + u32 ufid_flags;
>> + int err;
>> +
>> + err = nlmsg_parse(cb->nlh, GENL_HDRLEN + dp_flow_genl_family.hdrsize,
>> + a, dp_flow_genl_family.maxattr, flow_policy);
>
> Can you add genl helper function for this?
OK.
>> @@ -1235,6 +1304,8 @@ static const struct nla_policy flow_policy[OVS_FLOW_ATTR_MAX + 1] = {
>> [OVS_FLOW_ATTR_ACTIONS] = { .type = NLA_NESTED },
>> [OVS_FLOW_ATTR_CLEAR] = { .type = NLA_FLAG },
>> [OVS_FLOW_ATTR_PROBE] = { .type = NLA_FLAG },
>> + [OVS_FLOW_ATTR_UFID] = { .type = NLA_UNSPEC },
>> + [OVS_FLOW_ATTR_UFID_FLAGS] = { .type = NLA_U32 },
>> };
>>
>> static const struct genl_ops dp_flow_genl_ops[] = {
>> diff --git a/net/openvswitch/flow.h b/net/openvswitch/flow.h
>> index a8b30f3..7f31dbf 100644
>> --- a/net/openvswitch/flow.h
>> +++ b/net/openvswitch/flow.h
>> @@ -197,6 +197,13 @@ struct sw_flow_match {
>> struct sw_flow_mask *mask;
>> };
>>
>> +#define MAX_UFID_LENGTH 256
>> +
> For now we can limit this to 128 bits.
Is there a reason beyond trying to avoid the warning in flow_cmd_set()?
I suppose that its purpose as an identifier means that it's unlikely to
need to be much bigger in future (indeed, the larger it gets, the more
it would trade off the performance gains from using it).
>> @@ -213,13 +220,16 @@ struct flow_stats {
>>
>> struct sw_flow {
>> struct rcu_head rcu;
>> - struct hlist_node hash_node[2];
>> - u32 hash;
>> + struct {
>> + struct hlist_node node[2];
>> + u32 hash;
>> + } flow_hash, ufid_hash;
> I am not sure about _hash suffix here, Can you explain it? I think
> _table is better.
Agreed, table is better.
>> int stats_last_writer; /* NUMA-node id of the last writer on
>> * 'stats[0]'.
>> */
>> struct sw_flow_key key;
>> - struct sw_flow_key unmasked_key;
>> + struct sw_flow_id *ufid;
> Lets move this structure inside sw_flow, so that we can avoid one
> kmalloc during flow-install in case of UFID. something like:
>
> struct {
> u32 ufid_len;
> union {
> u32 ufid[MAX_UFID_LENGTH / 4];
> struct sw_flow_key *unmasked_key;
> }
> } id;
Agreed.
>> @@ -424,10 +475,9 @@ static struct sw_flow *masked_flow_lookup(struct table_instance *ti,
>> ovs_flow_mask_key(&masked_key, unmasked, mask);
>> hash = flow_hash(&masked_key, key_start, key_end);
>> head = find_bucket(ti, hash);
>> - hlist_for_each_entry_rcu(flow, head, hash_node[ti->node_ver]) {
>> - if (flow->mask == mask && flow->hash == hash &&
>> - flow_cmp_masked_key(flow, &masked_key,
>> - key_start, key_end))
>> + hlist_for_each_entry_rcu(flow, head, flow_hash.node[ti->node_ver]) {
>> + if (flow->mask == mask && flow->flow_hash.hash == hash &&
>> + flow_cmp_masked_key(flow, &masked_key, key_start, key_end))
>> return flow;
>> }
>> return NULL;
>> @@ -469,7 +519,40 @@ struct sw_flow *ovs_flow_tbl_lookup_exact(struct flow_table *tbl,
>> /* Always called under ovs-mutex. */
>> list_for_each_entry(mask, &tbl->mask_list, list) {
>> flow = masked_flow_lookup(ti, match->key, mask);
>> - if (flow && ovs_flow_cmp_unmasked_key(flow, match)) /* Found */
>> + if (flow && !flow->ufid &&
> why not NULL check for flow->unmasked_key here rather than ufid?
In this version, I tried to consistently use flow->ufid as the switch
for whether UFID exists or not. In the next version, this statement
would refer to flow->id->ufid_len.
The current approach means that ovs_flow_tbl_lookup_exact() is really
ovs_flow_tbl_lookup_unmasked_key(). Do you think this should remain
specific to unmasked key or should it be made to check that the
identifier (ufid OR unmasked key) is the same?
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
^ permalink raw reply
* Re: [PATCH iproute2 v2] ip link: Show devices by type
From: Stephen Hemminger @ 2014-12-10 0:31 UTC (permalink / raw)
To: Vadim Kochan; +Cc: netdev
In-Reply-To: <1417618598-23605-1-git-send-email-vadim4j@gmail.com>
On Wed, 3 Dec 2014 16:56:38 +0200
Vadim Kochan <vadim4j@gmail.com> wrote:
> Added new option 'type' to 'ip link show'
> command which allows to filter devices by type:
>
> ip link show type bridge
> ip link show type vlan
>
> Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
Does not apply to current iproute2 source.
Please update and resubmit
^ permalink raw reply
* Re: [patch iproute2 2/6] bridge/fdb: fix statistics output spacing
From: Stephen Hemminger @ 2014-12-10 0:32 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, nhorman, andy, tgraf, dborkman, ogerlitz, jesse,
pshelar, azhou, ben, jeffrey.t.kirsher, vyasevic, xiyou.wangcong,
john.r.fastabend, edumazet, jhs, sfeldma, f.fainelli, roopa,
linville, jasowang, ebiederm, nicolas.dichtel, ryazanov.s.a,
buytenh, aviadr, nbd, alexei.starovoitov, Neil.Jerram, ronye,
simon.horman, alexander.h.duyck, john.ronciak, mleitner, shrijeet,
gospo, bcrl, hemal
In-Reply-To: <1417683438-10935-3-git-send-email-jiri@resnulli.us>
On Thu, 4 Dec 2014 09:57:14 +0100
Jiri Pirko <jiri@resnulli.us> wrote:
> From: Scott Feldman <sfeldma@gmail.com>
>
> Signed-off-by: Scott Feldman <sfeldma@gmail.com>
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
Applied
^ permalink raw reply
* [net-next] dummy: use MODULE_VERSION
From: Flavio Leitner @ 2014-12-10 0:41 UTC (permalink / raw)
To: netdev; +Cc: Flavio Leitner
Use MODULE_VERSION() now that dummy driver has a version.
Signed-off-by: Flavio Leitner <fbl@redhat.com>
---
drivers/net/dummy.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/dummy.c b/drivers/net/dummy.c
index 413ca4f..49adbf1 100644
--- a/drivers/net/dummy.c
+++ b/drivers/net/dummy.c
@@ -225,3 +225,4 @@ module_init(dummy_init_module);
module_exit(dummy_cleanup_module);
MODULE_LICENSE("GPL");
MODULE_ALIAS_RTNL_LINK(DRV_NAME);
+MODULE_VERSION(DRV_VERSION);
--
1.9.3
^ permalink raw reply related
* [REGRESSION] iproute: change to if_bridge.h breaks build
From: Stephen Hemminger @ 2014-12-10 0:43 UTC (permalink / raw)
To: David Miller, Gregory Fong; +Cc: netdev
This commit causes build problems in iproute.
I can fix it for iproute, by reordering the headers
but on the principle that updates should never break
builds of existing code, this is wrong.
gcc -Wall -Wstrict-prototypes -Wmissing-prototypes -Wmissing-declarations -Wold-style-definition -Wformat=2 -O2 -I../include -DRESOLVE_HOSTNAMES -DLIBDIR=\"/usr/lib\" -DCONFDIR=\"/etc/iproute2\" -D_GNU_SOURCE -DHAVE_SETNS -c -o iplink_bridge_slave.o iplink_bridge_slave.c
In file included from ../include/linux/if_bridge.h:18:0,
from iplink_bridge_slave.c:16:
../include/linux/in6.h:169:0: warning: "IPV6_ADD_MEMBERSHIP" redefined
#define IPV6_ADD_MEMBERSHIP 20
^
In file included from /usr/include/netinet/in.h:37:0,
from iplink_bridge_slave.c:14:
commit 66f1c44887ba4f47d617f8ae21cf8e04e1892bd7
Author: Gregory Fong <gregory.0xf0@gmail.com>
Date: Tue Nov 4 11:21:21 2014 -0800
bridge: include in6.h in if_bridge.h for struct in6_addr
if_bridge.h uses struct in6_addr ip6, but wasn't including the in6.h
header. Thomas Backlund originally sent a patch to do this, but this
revealed a redefinition issue: https://lkml.org/lkml/2013/1/13/116
The redefinition issue should have been fixed by the following Linux
commits:
ee262ad827f89e2dc7851ec2986953b5b125c6bc inet: defines IPPROTO_* needed for module alias generation
cfd280c91253cc28e4919e349fa7a813b63e71e8 net: sync some IP headers with glibc
and the following glibc commit:
6c82a2f8d7c8e21e39237225c819f182ae438db3 Coordinate IPv6 definitions for Linux and glibc
so actually include the header now.
Reported-by: Colin Guthrie <colin@mageia.org>
Reported-by: Christiaan Welvaart <cjw@daneel.dyndns.org>
Reported-by: Thomas Backlund <tmb@mageia.org>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Cong Wang <xiyou.wangcong@gmail.com>
Cc: David Miller <davem@davemloft.net>
Signed-off-by: Gregory Fong <gregory.0xf0@gmail.com>
Acked-by: Cong Wang <xiyou.wangcong@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* Re: [PATCH] dummy: add support for ethtool get_drvinfo
From: Flavio Leitner @ 2014-12-10 0:44 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20141209.160726.984535504729715315.davem@davemloft.net>
On Tue, Dec 09, 2014 at 04:07:26PM -0500, David Miller wrote:
> From: Flavio Leitner <fbl@redhat.com>
> Date: Fri, 5 Dec 2014 22:13:24 -0200
>
> > The command 'ethtool -i' is useful to find details
> > about the interface like the device driver being used.
> > This was missing for dummy driver.
> >
> > Signed-off-by: Flavio Leitner <fbl@redhat.com>
>
> Applied, thank you.
>
> Consider adding a MODULE_VERSION instance since you've added an
> explicit version.
Done, patch sent:
[net-next] dummy: use MODULE_VERSION
fbl
^ permalink raw reply
* linux-next: manual merge of the net-next tree with the net tree
From: Stephen Rothwell @ 2014-12-10 1:20 UTC (permalink / raw)
To: David Miller, netdev, Lendacky, Thomas; +Cc: linux-next, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1658 bytes --]
Hi all,
Today's linux-next merge of the net-next tree got a conflict in
drivers/net/ethernet/amd/xgbe/xgbe-desc.c between commit 03ccc4c0a9da
("amd-xgbe: Do not clear interrupt indicator") from the net tree and
commit c9f140ebb008 ("amd-xgbe: Separate Tx/Rx ring data fields into
new structs") from the net-next tree.
I fixed it up (see below) and can carry the fix as necessary (no action
is required).
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
diff --cc drivers/net/ethernet/amd/xgbe/xgbe-desc.c
index b15551bad7fa,51b68d1299fe..000000000000
--- a/drivers/net/ethernet/amd/xgbe/xgbe-desc.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-desc.c
@@@ -354,8 -450,30 +450,29 @@@ static void xgbe_unmap_rdata(struct xgb
rdata->skb = NULL;
}
- rdata->tso_header = 0;
- rdata->len = 0;
+ if (rdata->rx.hdr.pa.pages)
+ put_page(rdata->rx.hdr.pa.pages);
+
+ if (rdata->rx.hdr.pa_unmap.pages) {
+ dma_unmap_page(pdata->dev, rdata->rx.hdr.pa_unmap.pages_dma,
+ rdata->rx.hdr.pa_unmap.pages_len,
+ DMA_FROM_DEVICE);
+ put_page(rdata->rx.hdr.pa_unmap.pages);
+ }
+
+ if (rdata->rx.buf.pa.pages)
+ put_page(rdata->rx.buf.pa.pages);
+
+ if (rdata->rx.buf.pa_unmap.pages) {
+ dma_unmap_page(pdata->dev, rdata->rx.buf.pa_unmap.pages_dma,
+ rdata->rx.buf.pa_unmap.pages_len,
+ DMA_FROM_DEVICE);
+ put_page(rdata->rx.buf.pa_unmap.pages);
+ }
+
+ memset(&rdata->tx, 0, sizeof(rdata->tx));
+ memset(&rdata->rx, 0, sizeof(rdata->rx));
+
- rdata->interrupt = 0;
rdata->mapped_as_page = 0;
if (rdata->state_saved) {
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* [PATCH net v4 0/7] cxgb4/cxgbi: misc. fixes for cxgb4i
From: Karen Xie @ 2014-12-10 1:32 UTC (permalink / raw)
To: linux-scsi, netdev
Cc: kxie, hariprasad, anish, hch, James.Bottomley, michaelc, davem
[PATCH net v4 0/7] cxgb4/cxgbi: misc. fixes for cxgb4i
This patch set fixes cxgb4i's tx credit calculation and adds handling of additional rx message and negative advice types. It also removes the duplicate code in cxgb4i to set the outgoing queues of a packet.
Karen Xie (7):
cxgb4i: fix tx immediate data credit check
cxgb4i: fix credit check for tx_data_wr
cxgb4/cxgb4i: set max. outgoing pdu length in the f/w
cxgb4i: add more types of negative advice
cxgb4i: handle non pdu-aligned rx data
cxgb4i: use cxgb4's set_wr_txq() for setting outgoing queues
libcxgbi: fix the debug print accessing skb after it is freed
Sending to net as the fixes are mostly in the network area and it touches cxgb4's header file (t4fw_api.h).
v2 corrects the "CHECK"s flagged by checkpatch.pl --strict.
v3 splits the 3rd patch from v2 to two seperate patches. Adds detailed commit messages and makes subject more concise. Patch 3/6 also changes the return value of is_neg_adv() from int to bool.
v4 splits the 1st patch from v3 to two seperate patches and reduces code duplication in make_tx_data_wr().
^ permalink raw reply
* [PATCH net v4 1/7] cxgb4i: fix tx immediate data credit check
From: Karen Xie @ 2014-12-10 1:32 UTC (permalink / raw)
To: linux-scsi, netdev
Cc: kxie, hariprasad, anish, hch, James.Bottomley, michaelc, davem
[PATCH net v4 0/7] cxgb4/cxgbi: misc. fixes for cxgb4i
This patch set fixes cxgb4i's tx credit calculation and adds handling of additional rx message and negative advice types. It also removes the duplicate code in cxgb4i to set the outgoing queues of a packet.
Karen Xie (7):
cxgb4i: fix tx immediate data credit check
cxgb4i: fix credit check for tx_data_wr
cxgb4/cxgb4i: set max. outgoing pdu length in the f/w
cxgb4i: add more types of negative advice
cxgb4i: handle non pdu-aligned rx data
cxgb4i: use cxgb4's set_wr_txq() for setting outgoing queues
libcxgbi: fix the debug print accessing skb after it is freed
Sending to net as the fixes are mostly in the network area and it touches cxgb4's header file (t4fw_api.h).
v2 corrects the "CHECK"s flagged by checkpatch.pl --strict.
v3 splits the 3rd patch from v2 to two seperate patches. Adds detailed commit messages and makes subject more concise. Patch 3/6 also changes the return value of is_neg_adv() from int to bool.
v4 splits the 1st patch from v3 to two seperate patches and reduces code duplication in make_tx_data_wr().
^ permalink raw reply
* [PATCH net v4 7/7] libcxgbi: free skb after debug prints
From: Karen Xie @ 2014-12-10 1:33 UTC (permalink / raw)
To: linux-scsi, netdev
Cc: kxie, hariprasad, anish, hch, James.Bottomley, michaelc, davem
[PATCH net v4 7/7] libcxgbi: free skb after debug prints
From: Karen Xie <kxie@chelsio.com>
The debug print was accessing the skb after it was freed.
Signed-off-by: Karen Xie <kxie@chelsio.com>
---
drivers/scsi/cxgbi/libcxgbi.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/drivers/scsi/cxgbi/libcxgbi.c b/drivers/scsi/cxgbi/libcxgbi.c
index 7da59c3..eb58afc 100644
--- a/drivers/scsi/cxgbi/libcxgbi.c
+++ b/drivers/scsi/cxgbi/libcxgbi.c
@@ -2294,10 +2294,12 @@ int cxgbi_conn_xmit_pdu(struct iscsi_task *task)
return err;
}
- kfree_skb(skb);
log_debug(1 << CXGBI_DBG_ISCSI | 1 << CXGBI_DBG_PDU_TX,
"itt 0x%x, skb 0x%p, len %u/%u, xmit err %d.\n",
task->itt, skb, skb->len, skb->data_len, err);
+
+ kfree_skb(skb);
+
iscsi_conn_printk(KERN_ERR, task->conn, "xmit err %d.\n", err);
iscsi_conn_failure(task->conn, ISCSI_ERR_XMIT_FAILED);
return err;
^ permalink raw reply related
* [PATCH v2] net: introduce helper macro for_each_cmsghdr
From: Gu Zheng @ 2014-12-10 1:39 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, linux-kernel, Joe Perches
Introduce helper macro for_each_cmsghdr as a wrapper of the enumerating
cmsghdr from msghdr, just cleanup.
Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
---
v2: use the lower-case macro name as Joe suggested.
---
.../networking/timestamping/timestamping.c | 4 +---
.../networking/timestamping/txtimestamp.c | 4 +---
crypto/af_alg.c | 2 +-
include/linux/socket.h | 4 ++++
net/core/scm.c | 3 +--
net/dccp/proto.c | 5 ++---
net/ipv4/ip_sockglue.c | 2 +-
net/ipv6/datagram.c | 2 +-
net/iucv/af_iucv.c | 4 +---
net/rds/send.c | 4 ++--
net/rxrpc/ar-output.c | 2 +-
net/sctp/socket.c | 3 +--
12 files changed, 17 insertions(+), 22 deletions(-)
diff --git a/Documentation/networking/timestamping/timestamping.c b/Documentation/networking/timestamping/timestamping.c
index 5cdfd74..3106e88 100644
--- a/Documentation/networking/timestamping/timestamping.c
+++ b/Documentation/networking/timestamping/timestamping.c
@@ -169,9 +169,7 @@ static void printpacket(struct msghdr *msg, int res,
res,
inet_ntoa(from_addr->sin_addr),
msg->msg_controllen);
- for (cmsg = CMSG_FIRSTHDR(msg);
- cmsg;
- cmsg = CMSG_NXTHDR(msg, cmsg)) {
+ for_each_cmsghdr(cmsg, msg) {
printf(" cmsg len %zu: ", cmsg->cmsg_len);
switch (cmsg->cmsg_level) {
case SOL_SOCKET:
diff --git a/Documentation/networking/timestamping/txtimestamp.c b/Documentation/networking/timestamping/txtimestamp.c
index b32fc2a..e44ef35 100644
--- a/Documentation/networking/timestamping/txtimestamp.c
+++ b/Documentation/networking/timestamping/txtimestamp.c
@@ -149,9 +149,7 @@ static void __recv_errmsg_cmsg(struct msghdr *msg, int payload_len)
struct scm_timestamping *tss = NULL;
struct cmsghdr *cm;
- for (cm = CMSG_FIRSTHDR(msg);
- cm && cm->cmsg_len;
- cm = CMSG_NXTHDR(msg, cm)) {
+ for_each_cmsghdr(cmsg, msg) {
if (cm->cmsg_level == SOL_SOCKET &&
cm->cmsg_type == SCM_TIMESTAMPING) {
tss = (void *) CMSG_DATA(cm);
diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index 6a3ad80..3df7d53 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -399,7 +399,7 @@ int af_alg_cmsg_send(struct msghdr *msg, struct af_alg_control *con)
{
struct cmsghdr *cmsg;
- for (cmsg = CMSG_FIRSTHDR(msg); cmsg; cmsg = CMSG_NXTHDR(msg, cmsg)) {
+ for_each_cmsghdr(cmsg, msg) {
if (!CMSG_OK(msg, cmsg))
return -EINVAL;
if (cmsg->cmsg_level != SOL_ALG)
diff --git a/include/linux/socket.h b/include/linux/socket.h
index bb9b836..d4b592f 100644
--- a/include/linux/socket.h
+++ b/include/linux/socket.h
@@ -94,6 +94,10 @@ struct cmsghdr {
(cmsg)->cmsg_len <= (unsigned long) \
((mhdr)->msg_controllen - \
((char *)(cmsg) - (char *)(mhdr)->msg_control)))
+#define for_each_cmsg_hdr(cmsg, msg) \
+ for (cmsg = CMSG_FIRSTHDR(msg); \
+ cmsg; \
+ cmsg = CMSG_NXTHDR(msg, cmsg))
/*
* Get the next cmsg header
diff --git a/net/core/scm.c b/net/core/scm.c
index b442e7e..e938c49 100644
--- a/net/core/scm.c
+++ b/net/core/scm.c
@@ -129,8 +129,7 @@ int __scm_send(struct socket *sock, struct msghdr *msg, struct scm_cookie *p)
struct cmsghdr *cmsg;
int err;
- for (cmsg = CMSG_FIRSTHDR(msg); cmsg; cmsg = CMSG_NXTHDR(msg, cmsg))
- {
+ for_each_cmsghdr(cmsg, msg) {
err = -EINVAL;
/* Verify that cmsg_len is at least sizeof(struct cmsghdr) */
diff --git a/net/dccp/proto.c b/net/dccp/proto.c
index 5ab6627..d449cc5 100644
--- a/net/dccp/proto.c
+++ b/net/dccp/proto.c
@@ -703,7 +703,7 @@ EXPORT_SYMBOL_GPL(compat_dccp_getsockopt);
static int dccp_msghdr_parse(struct msghdr *msg, struct sk_buff *skb)
{
- struct cmsghdr *cmsg = CMSG_FIRSTHDR(msg);
+ struct cmsghdr *cmsg;
/*
* Assign an (opaque) qpolicy priority value to skb->priority.
@@ -717,8 +717,7 @@ static int dccp_msghdr_parse(struct msghdr *msg, struct sk_buff *skb)
*/
skb->priority = 0;
- for (; cmsg != NULL; cmsg = CMSG_NXTHDR(msg, cmsg)) {
-
+ for_each_cmsghdr(cmsg, msg) {
if (!CMSG_OK(msg, cmsg))
return -EINVAL;
diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
index 9daf217..14a6f71 100644
--- a/net/ipv4/ip_sockglue.c
+++ b/net/ipv4/ip_sockglue.c
@@ -192,7 +192,7 @@ int ip_cmsg_send(struct net *net, struct msghdr *msg, struct ipcm_cookie *ipc,
int err, val;
struct cmsghdr *cmsg;
- for (cmsg = CMSG_FIRSTHDR(msg); cmsg; cmsg = CMSG_NXTHDR(msg, cmsg)) {
+ for_each_cmsghdr(cmsg, msg) {
if (!CMSG_OK(msg, cmsg))
return -EINVAL;
#if IS_ENABLED(CONFIG_IPV6)
diff --git a/net/ipv6/datagram.c b/net/ipv6/datagram.c
index 2cdc383..9895b98 100644
--- a/net/ipv6/datagram.c
+++ b/net/ipv6/datagram.c
@@ -640,7 +640,7 @@ int ip6_datagram_send_ctl(struct net *net, struct sock *sk,
int len;
int err = 0;
- for (cmsg = CMSG_FIRSTHDR(msg); cmsg; cmsg = CMSG_NXTHDR(msg, cmsg)) {
+ for_each_cmsghdr(cmsg, msg) {
int addr_type;
if (!CMSG_OK(msg, cmsg)) {
diff --git a/net/iucv/af_iucv.c b/net/iucv/af_iucv.c
index a089b6b..eae4e08 100644
--- a/net/iucv/af_iucv.c
+++ b/net/iucv/af_iucv.c
@@ -1070,9 +1070,7 @@ static int iucv_sock_sendmsg(struct kiocb *iocb, struct socket *sock,
txmsg.class = 0;
/* iterate over control messages */
- for (cmsg = CMSG_FIRSTHDR(msg); cmsg;
- cmsg = CMSG_NXTHDR(msg, cmsg)) {
-
+ for_each_cmsghdr(cmsg, msg) {
if (!CMSG_OK(msg, cmsg)) {
err = -EINVAL;
goto out;
diff --git a/net/rds/send.c b/net/rds/send.c
index 0a64541..1accb3e 100644
--- a/net/rds/send.c
+++ b/net/rds/send.c
@@ -826,7 +826,7 @@ static int rds_rm_size(struct msghdr *msg, int data_len)
int cmsg_groups = 0;
int retval;
- for (cmsg = CMSG_FIRSTHDR(msg); cmsg; cmsg = CMSG_NXTHDR(msg, cmsg)) {
+ for_each_cmsghdr(cmsg, msg) {
if (!CMSG_OK(msg, cmsg))
return -EINVAL;
@@ -878,7 +878,7 @@ static int rds_cmsg_send(struct rds_sock *rs, struct rds_message *rm,
struct cmsghdr *cmsg;
int ret = 0;
- for (cmsg = CMSG_FIRSTHDR(msg); cmsg; cmsg = CMSG_NXTHDR(msg, cmsg)) {
+ for_each_cmsghdr(cmsg, msg) {
if (!CMSG_OK(msg, cmsg))
return -EINVAL;
diff --git a/net/rxrpc/ar-output.c b/net/rxrpc/ar-output.c
index 0b4b9a7..f915e7e 100644
--- a/net/rxrpc/ar-output.c
+++ b/net/rxrpc/ar-output.c
@@ -45,7 +45,7 @@ static int rxrpc_sendmsg_cmsg(struct rxrpc_sock *rx, struct msghdr *msg,
if (msg->msg_controllen == 0)
return -EINVAL;
- for (cmsg = CMSG_FIRSTHDR(msg); cmsg; cmsg = CMSG_NXTHDR(msg, cmsg)) {
+ for_each_cmsghdr(cmsg, msg) {
if (!CMSG_OK(msg, cmsg))
return -EINVAL;
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 634a2ab..58834d7 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -6592,8 +6592,7 @@ static int sctp_msghdr_parse(const struct msghdr *msg, sctp_cmsgs_t *cmsgs)
struct cmsghdr *cmsg;
struct msghdr *my_msg = (struct msghdr *)msg;
- for (cmsg = CMSG_FIRSTHDR(msg); cmsg != NULL;
- cmsg = CMSG_NXTHDR(my_msg, cmsg)) {
+ for_each_cmsghdr(cmsg, my_msg) {
if (!CMSG_OK(my_msg, cmsg))
return -EINVAL;
--
1.7.7
^ permalink raw reply related
* RE: [PATCH net v3 1/6] cxgb4i: fix tx credit calculation
From: Karen Xie @ 2014-12-10 1:38 UTC (permalink / raw)
To: Sergei Shtylyov, linux-scsi@vger.kernel.org,
netdev@vger.kernel.org
Cc: Hariprasad S, Anish Bhatt, hch@infradead.org,
James.Bottomley@HansenPartnership.com, michaelc@cs.wisc.edu,
davem@davemloft.net
In-Reply-To: <54875385.80708@cogentembedded.com>
Thanks for the review comment, have sent the v4 set to incorporate your comments.
-----Original Message-----
From: Sergei Shtylyov [mailto:sergei.shtylyov@cogentembedded.com]
Sent: Tuesday, December 09, 2014 11:55 AM
To: Karen Xie; linux-scsi@vger.kernel.org; netdev@vger.kernel.org
Cc: Hariprasad S; Anish Bhatt; hch@infradead.org; James.Bottomley@HansenPartnership.com; michaelc@cs.wisc.edu; davem@davemloft.net
Subject: Re: [PATCH net v3 1/6] cxgb4i: fix tx credit calculation
Hello.
On 12/09/2014 08:32 PM, Karen Xie wrote:
> [PATCH net v3 1/6] cxgb4i: fix tx credit calculation
> From: Karen Xie <kxie@chelsio.com>
> - Only data skbs need the wr header added while control skbs do not. Make sure they are treated differently.
> - Any credit related checking should be done before adding the wr header.
Looks like a spearate issue deserving its own patch?
> - Fixed compiler warning resulted from added cxgbi_skb_test_flag() call in is_ofld_imm().
Isn't it called cxgbi_skcb_test_flag()?
> Signed-off-by: Karen Xie <kxie@chelsio.com>
> ---
> drivers/scsi/cxgbi/cxgb4i/cxgb4i.c | 26 +++++++++++++++++---------
> drivers/scsi/cxgbi/libcxgbi.h | 4 ++--
> 2 files changed, 19 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
> b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
> index 1508125..5c3f15d 100644
> --- a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
> +++ b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
[...]
> @@ -544,15 +548,17 @@ static inline void make_tx_data_wr(struct cxgbi_sock *csk, struct sk_buff *skb,
> unsigned int submode = cxgbi_skcb_ulp_mode(skb) & 3;
> unsigned int wr_ulp_mode = 0;
>
> - req = (struct fw_ofld_tx_data_wr *)__skb_push(skb, sizeof(*req));
> -
Perhaps it makes sense to store the result of is_ofld_imm() before
__skb_push() call
instead of duplicating the same call in 2 branches?
> if (is_ofld_imm(skb)) {
> + req = (struct fw_ofld_tx_data_wr *)__skb_push(skb,
> + sizeof(*req));
The continuation line should start right under 'skb' on the previous line.
> req->op_to_immdlen = htonl(FW_WR_OP(FW_OFLD_TX_DATA_WR) |
> FW_WR_COMPL(1) |
> FW_WR_IMMDLEN(dlen));
> req->flowid_len16 = htonl(FW_WR_FLOWID(csk->tid) |
> FW_WR_LEN16(credits));
> } else {
> + req = (struct fw_ofld_tx_data_wr *)__skb_push(skb,
> + sizeof(*req));
Likewise.
> req->op_to_immdlen =
> cpu_to_be32(FW_WR_OP(FW_OFLD_TX_DATA_WR) |
> FW_WR_COMPL(1) |
> @@ -597,12 +603,14 @@ static int push_tx_frames(struct cxgbi_sock
> *csk, int req_completion)
>
> skb_reset_transport_header(skb);
> if (is_ofld_imm(skb))
> - credits_needed = DIV_ROUND_UP(dlen +
> - sizeof(struct fw_ofld_tx_data_wr), 16);
> + credits_needed = DIV_ROUND_UP(dlen, 16);
> else
> - credits_needed = DIV_ROUND_UP(8*calc_tx_flits_ofld(skb)
> - + sizeof(struct fw_ofld_tx_data_wr),
> - 16);
> + credits_needed = DIV_ROUND_UP(8*calc_tx_flits_ofld(skb),
It would have been good if you added spaces around *, while at it, to keep it consistent with the general kernel coding style...
[...]
WBR, Sergei
^ permalink raw reply
* [PATCH net v5 3/7] cxgb4/cxgb4i: set the max. pdu length in firmware
From: Karen Xie @ 2014-12-10 1:43 UTC (permalink / raw)
To: linux-scsi, netdev
Cc: kxie, hariprasad, anish, hch, James.Bottomley, michaelc, davem
[PATCH net v5 3/7] cxgb4/cxgb4i: set the max. pdu length in firmware.
From: Karen Xie <kxie@chelsio.com>
Programs the firmware of the maximum outgoing iscsi pdu length per connection.
Signed-off-by: Karen Xie <kxie@chelsio.com>
---
drivers/net/ethernet/chelsio/cxgb4/t4fw_api.h | 1
drivers/scsi/cxgbi/cxgb4i/cxgb4i.c | 69 ++++++++++++++++++-------
2 files changed, 52 insertions(+), 18 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/t4fw_api.h b/drivers/net/ethernet/chelsio/cxgb4/t4fw_api.h
index 3409756..743a350 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/t4fw_api.h
+++ b/drivers/net/ethernet/chelsio/cxgb4/t4fw_api.h
@@ -529,6 +529,7 @@ enum fw_flowc_mnem {
FW_FLOWC_MNEM_RCVNXT,
FW_FLOWC_MNEM_SNDBUF,
FW_FLOWC_MNEM_MSS,
+ FW_FLOWC_MNEM_TXDATAPLEN_MAX,
};
struct fw_flowc_mnemval {
diff --git a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
index 56dbd25..86b9bbc 100644
--- a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
+++ b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
@@ -75,6 +75,7 @@ typedef void (*cxgb4i_cplhandler_func)(struct cxgbi_device *, struct sk_buff *);
static void *t4_uld_add(const struct cxgb4_lld_info *);
static int t4_uld_rx_handler(void *, const __be64 *, const struct pkt_gl *);
static int t4_uld_state_change(void *, enum cxgb4_state state);
+static inline int send_tx_flowc_wr(struct cxgbi_sock *);
static const struct cxgb4_uld_info cxgb4i_uld_info = {
.name = DRV_MODULE_NAME,
@@ -391,6 +392,12 @@ static void send_abort_req(struct cxgbi_sock *csk)
if (unlikely(csk->state == CTP_ABORTING) || !skb || !csk->cdev)
return;
+
+ if (!cxgbi_sock_flag(csk, CTPF_TX_DATA_SENT)) {
+ send_tx_flowc_wr(csk);
+ cxgbi_sock_set_flag(csk, CTPF_TX_DATA_SENT);
+ }
+
cxgbi_sock_set_state(csk, CTP_ABORTING);
cxgbi_sock_set_flag(csk, CTPF_ABORT_RPL_PENDING);
cxgbi_sock_purge_write_queue(csk);
@@ -493,20 +500,40 @@ static inline unsigned int calc_tx_flits_ofld(const struct sk_buff *skb)
return flits + sgl_len(cnt);
}
-static inline void send_tx_flowc_wr(struct cxgbi_sock *csk)
+#define FLOWC_WR_NPARAMS_MIN 9
+static inline int tx_flowc_wr_credits(int *nparamsp, int *flowclenp)
+{
+ int nparams, flowclen16, flowclen;
+
+ nparams = FLOWC_WR_NPARAMS_MIN;
+ flowclen = offsetof(struct fw_flowc_wr, mnemval[nparams]);
+ flowclen16 = DIV_ROUND_UP(flowclen, 16);
+ flowclen = flowclen16 * 16;
+ /*
+ * Return the number of 16-byte credits used by the FlowC request.
+ * Pass back the nparams and actual FlowC length if requested.
+ */
+ if (nparamsp)
+ *nparamsp = nparams;
+ if (flowclenp)
+ *flowclenp = flowclen;
+
+ return flowclen16;
+}
+
+static inline int send_tx_flowc_wr(struct cxgbi_sock *csk)
{
struct sk_buff *skb;
struct fw_flowc_wr *flowc;
- int flowclen, i;
+ int nparams, flowclen16, flowclen;
- flowclen = 80;
+ flowclen16 = tx_flowc_wr_credits(&nparams, &flowclen);
skb = alloc_wr(flowclen, 0, GFP_ATOMIC);
flowc = (struct fw_flowc_wr *)skb->head;
flowc->op_to_nparams =
- htonl(FW_WR_OP(FW_FLOWC_WR) | FW_FLOWC_WR_NPARAMS(8));
+ htonl(FW_WR_OP(FW_FLOWC_WR) | FW_FLOWC_WR_NPARAMS(nparams));
flowc->flowid_len16 =
- htonl(FW_WR_LEN16(DIV_ROUND_UP(72, 16)) |
- FW_WR_FLOWID(csk->tid));
+ htonl(FW_WR_LEN16(flowclen16) | FW_WR_FLOWID(csk->tid));
flowc->mnemval[0].mnemonic = FW_FLOWC_MNEM_PFNVFN;
flowc->mnemval[0].val = htonl(csk->cdev->pfvf);
flowc->mnemval[1].mnemonic = FW_FLOWC_MNEM_CH;
@@ -525,11 +552,9 @@ static inline void send_tx_flowc_wr(struct cxgbi_sock *csk)
flowc->mnemval[7].val = htonl(csk->advmss);
flowc->mnemval[8].mnemonic = 0;
flowc->mnemval[8].val = 0;
- for (i = 0; i < 9; i++) {
- flowc->mnemval[i].r4[0] = 0;
- flowc->mnemval[i].r4[1] = 0;
- flowc->mnemval[i].r4[2] = 0;
- }
+ flowc->mnemval[8].mnemonic = FW_FLOWC_MNEM_TXDATAPLEN_MAX;
+ flowc->mnemval[8].val = 16384;
+
set_queue(skb, CPL_PRIORITY_DATA, csk);
log_debug(1 << CXGBI_DBG_TOE | 1 << CXGBI_DBG_SOCK,
@@ -539,6 +564,8 @@ static inline void send_tx_flowc_wr(struct cxgbi_sock *csk)
csk->advmss);
cxgb4_ofld_send(csk->cdev->ports[csk->port_id], skb);
+
+ return flowclen16;
}
static inline void make_tx_data_wr(struct cxgbi_sock *csk, struct sk_buff *skb,
@@ -599,6 +626,7 @@ static int push_tx_frames(struct cxgbi_sock *csk, int req_completion)
int dlen = skb->len;
int len = skb->len;
unsigned int credits_needed;
+ int flowclen16 = 0;
skb_reset_transport_header(skb);
if (is_ofld_imm(skb))
@@ -613,6 +641,17 @@ static int push_tx_frames(struct cxgbi_sock *csk, int req_completion)
sizeof(struct fw_ofld_tx_data_wr),
16);
+ /*
+ * Assumes the initial credits is large enough to support
+ * fw_flowc_wr plus largest possible first payload
+ */
+ if (!cxgbi_sock_flag(csk, CTPF_TX_DATA_SENT)) {
+ flowclen16 = send_tx_flowc_wr(csk);
+ csk->wr_cred -= flowclen16;
+ csk->wr_una_cred += flowclen16;
+ cxgbi_sock_set_flag(csk, CTPF_TX_DATA_SENT);
+ }
+
if (csk->wr_cred < credits_needed) {
log_debug(1 << CXGBI_DBG_PDU_TX,
"csk 0x%p, skb %u/%u, wr %d < %u.\n",
@@ -622,7 +661,7 @@ static int push_tx_frames(struct cxgbi_sock *csk, int req_completion)
}
__skb_unlink(skb, &csk->write_queue);
set_queue(skb, CPL_PRIORITY_DATA, csk);
- skb->csum = credits_needed;
+ skb->csum = credits_needed + flowclen16;
csk->wr_cred -= credits_needed;
csk->wr_una_cred += credits_needed;
cxgbi_sock_enqueue_wr(csk, skb);
@@ -633,12 +672,6 @@ static int push_tx_frames(struct cxgbi_sock *csk, int req_completion)
csk->wr_cred, csk->wr_una_cred);
if (likely(cxgbi_skcb_test_flag(skb, SKCBF_TX_NEED_HDR))) {
- if (!cxgbi_sock_flag(csk, CTPF_TX_DATA_SENT)) {
- send_tx_flowc_wr(csk);
- skb->csum += 5;
- csk->wr_cred -= 5;
- csk->wr_una_cred += 5;
- }
len += cxgbi_ulp_extra_len(cxgbi_skcb_ulp_mode(skb));
make_tx_data_wr(csk, skb, dlen, len, credits_needed,
req_completion);
^ permalink raw reply related
* [PATCH net v5 6/7] cxgb4i: use set_wr_txq() to set tx queues
From: Karen Xie @ 2014-12-10 1:43 UTC (permalink / raw)
To: linux-scsi, netdev
Cc: kxie, hariprasad, anish, hch, James.Bottomley, michaelc, davem
[PATCH net v5 6/7] cxgb4i: use set_wr_txq() to set tx queues
From: Karen Xie <kxie@chelsio.com>
use cxgb4's set_wr_txq() for setting of the tx queue for a outgoing packet. Remove the similar function in cxgb4i.
Signed-off-by: Karen Xie <kxie@chelsio.com>
---
drivers/scsi/cxgbi/cxgb4i/cxgb4i.c | 16 +++++-----------
1 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
index bc66308..03a0afe 100644
--- a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
+++ b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
@@ -157,12 +157,6 @@ static struct scsi_transport_template *cxgb4i_stt;
#define RCV_BUFSIZ_MASK 0x3FFU
#define MAX_IMM_TX_PKT_LEN 128
-static inline void set_queue(struct sk_buff *skb, unsigned int queue,
- const struct cxgbi_sock *csk)
-{
- skb->queue_mapping = queue;
-}
-
static int push_tx_frames(struct cxgbi_sock *, int);
/*
@@ -404,7 +398,7 @@ static void send_abort_req(struct cxgbi_sock *csk)
csk->cpl_abort_req = NULL;
req = (struct cpl_abort_req *)skb->head;
- set_queue(skb, CPL_PRIORITY_DATA, csk);
+ set_wr_txq(skb, CPL_PRIORITY_DATA, csk->port_id);
req->cmd = CPL_ABORT_SEND_RST;
t4_set_arp_err_handler(skb, csk, abort_arp_failure);
INIT_TP_WR(req, csk->tid);
@@ -430,7 +424,7 @@ static void send_abort_rpl(struct cxgbi_sock *csk, int rst_status)
csk, csk->state, csk->flags, csk->tid, rst_status);
csk->cpl_abort_rpl = NULL;
- set_queue(skb, CPL_PRIORITY_DATA, csk);
+ set_wr_txq(skb, CPL_PRIORITY_DATA, csk->port_id);
INIT_TP_WR(rpl, csk->tid);
OPCODE_TID(rpl) = cpu_to_be32(MK_OPCODE_TID(CPL_ABORT_RPL, csk->tid));
rpl->cmd = rst_status;
@@ -555,7 +549,7 @@ static inline int send_tx_flowc_wr(struct cxgbi_sock *csk)
flowc->mnemval[8].mnemonic = FW_FLOWC_MNEM_TXDATAPLEN_MAX;
flowc->mnemval[8].val = 16384;
- set_queue(skb, CPL_PRIORITY_DATA, csk);
+ set_wr_txq(skb, CPL_PRIORITY_DATA, csk->port_id);
log_debug(1 << CXGBI_DBG_TOE | 1 << CXGBI_DBG_SOCK,
"csk 0x%p, tid 0x%x, %u,%u,%u,%u,%u,%u,%u.\n",
@@ -660,7 +654,7 @@ static int push_tx_frames(struct cxgbi_sock *csk, int req_completion)
break;
}
__skb_unlink(skb, &csk->write_queue);
- set_queue(skb, CPL_PRIORITY_DATA, csk);
+ set_wr_txq(skb, CPL_PRIORITY_DATA, csk->port_id);
skb->csum = credits_needed + flowclen16;
csk->wr_cred -= credits_needed;
csk->wr_una_cred += credits_needed;
@@ -1552,7 +1546,7 @@ static int ddp_ppod_write_idata(struct cxgbi_device *cdev, unsigned int port_id,
return -ENOMEM;
}
req = (struct ulp_mem_io *)skb->head;
- set_queue(skb, CPL_PRIORITY_CONTROL, NULL);
+ set_wr_txq(skb, CPL_PRIORITY_CONTROL, 0);
ulp_mem_io_set_hdr(lldi, req, wr_len, dlen, pm_addr);
idata = (struct ulptx_idata *)(req + 1);
^ permalink raw reply related
* [PATCH net v5 1/7] cxgb4i: fix tx immediate data credit check
From: Karen Xie @ 2014-12-10 1:43 UTC (permalink / raw)
To: linux-scsi, netdev
Cc: kxie, hariprasad, anish, hch, James.Bottomley, michaelc, davem
[PATCH net v5 1/7] cxgb4i: fix tx immediate data credit check
From: Karen Xie <kxie@chelsio.com>
Only data skbs need the wr header added while control skbs do not. Make sure they are treated differently.
Signed-off-by: Karen Xie <kxie@chelsio.com>
---
drivers/scsi/cxgbi/cxgb4i/cxgb4i.c | 22 +++++++++++++++-------
drivers/scsi/cxgbi/libcxgbi.h | 4 ++--
2 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
index 1508125..f119a67 100644
--- a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
+++ b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
@@ -171,10 +171,14 @@ static int push_tx_frames(struct cxgbi_sock *, int);
* Returns true if a packet can be sent as an offload WR with immediate
* data. We currently use the same limit as for Ethernet packets.
*/
-static inline int is_ofld_imm(const struct sk_buff *skb)
+static inline bool is_ofld_imm(const struct sk_buff *skb)
{
- return skb->len <= (MAX_IMM_TX_PKT_LEN -
- sizeof(struct fw_ofld_tx_data_wr));
+ int len = skb->len;
+
+ if (likely(cxgbi_skcb_test_flag(skb, SKCBF_TX_NEED_HDR)))
+ len += sizeof(struct fw_ofld_tx_data_wr);
+
+ return len <= MAX_IMM_TX_PKT_LEN;
}
static void send_act_open_req(struct cxgbi_sock *csk, struct sk_buff *skb,
@@ -597,11 +601,15 @@ static int push_tx_frames(struct cxgbi_sock *csk, int req_completion)
skb_reset_transport_header(skb);
if (is_ofld_imm(skb))
- credits_needed = DIV_ROUND_UP(dlen +
- sizeof(struct fw_ofld_tx_data_wr), 16);
+ credits_needed = DIV_ROUND_UP(dlen, 16);
else
- credits_needed = DIV_ROUND_UP(8*calc_tx_flits_ofld(skb)
- + sizeof(struct fw_ofld_tx_data_wr),
+ credits_needed = DIV_ROUND_UP(
+ 8 * calc_tx_flits_ofld(skb),
+ 16);
+
+ if (likely(cxgbi_skcb_test_flag(skb, SKCBF_TX_NEED_HDR)))
+ credits_needed += DIV_ROUND_UP(
+ sizeof(struct fw_ofld_tx_data_wr),
16);
if (csk->wr_cred < credits_needed) {
diff --git a/drivers/scsi/cxgbi/libcxgbi.h b/drivers/scsi/cxgbi/libcxgbi.h
index 2c7cb1c..aba1af7 100644
--- a/drivers/scsi/cxgbi/libcxgbi.h
+++ b/drivers/scsi/cxgbi/libcxgbi.h
@@ -317,8 +317,8 @@ static inline void cxgbi_skcb_clear_flag(struct sk_buff *skb,
__clear_bit(flag, &(cxgbi_skcb_flags(skb)));
}
-static inline int cxgbi_skcb_test_flag(struct sk_buff *skb,
- enum cxgbi_skcb_flags flag)
+static inline int cxgbi_skcb_test_flag(const struct sk_buff *skb,
+ enum cxgbi_skcb_flags flag)
{
return test_bit(flag, &(cxgbi_skcb_flags(skb)));
}
^ permalink raw reply related
* [PATCH net v5 0/7] cxgb4/cxgbi: misc. fixes for cxgb4i
From: Karen Xie @ 2014-12-10 1:43 UTC (permalink / raw)
To: linux-scsi, netdev
Cc: kxie, hariprasad, anish, hch, James.Bottomley, michaelc, davem
[PATCH net v5 0/7] cxgb4/cxgbi: misc. fixes for cxgb4i
This patch set fixes cxgb4i's tx credit calculation and adds handling of additional rx message and negative advice types. It also removes the duplicate code in cxgb4i to set the outgoing queues of a packet.
Karen Xie (7):
cxgb4i: fix tx immediate data credit check
cxgb4i: fix credit check for tx_data_wr
cxgb4/cxgb4i: set max. outgoing pdu length in the f/w
cxgb4i: add more types of negative advice
cxgb4i: handle non pdu-aligned rx data
cxgb4i: use cxgb4's set_wr_txq() for setting outgoing queues
libcxgbi: fix the debug print accessing skb after it is freed
Sending to net as the fixes are mostly in the network area and it touches cxgb4's header file (t4fw_api.h).
v2 corrects the "CHECK"s flagged by checkpatch.pl --strict.
v3 splits the 3rd patch from v2 to two seperate patches. Adds detailed commit messages and makes subject more concise. Patch 3/6 also changes the return value of is_neg_adv() from int to bool.
v4 -- please ignore.
v5 splits the 1st patch from v3 to two seperate patches and reduces code duplication in make_tx_data_wr().
^ permalink raw reply
* [PATCH net v5 7/7] libcxgbi: free skb after debug prints
From: Karen Xie @ 2014-12-10 1:43 UTC (permalink / raw)
To: linux-scsi, netdev
Cc: kxie, hariprasad, anish, hch, James.Bottomley, michaelc, davem
[PATCH net v5 7/7] libcxgbi: free skb after debug prints
From: Karen Xie <kxie@chelsio.com>
The debug print was accessing the skb after it was freed.
Signed-off-by: Karen Xie <kxie@chelsio.com>
---
drivers/scsi/cxgbi/libcxgbi.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/drivers/scsi/cxgbi/libcxgbi.c b/drivers/scsi/cxgbi/libcxgbi.c
index 7da59c3..eb58afc 100644
--- a/drivers/scsi/cxgbi/libcxgbi.c
+++ b/drivers/scsi/cxgbi/libcxgbi.c
@@ -2294,10 +2294,12 @@ int cxgbi_conn_xmit_pdu(struct iscsi_task *task)
return err;
}
- kfree_skb(skb);
log_debug(1 << CXGBI_DBG_ISCSI | 1 << CXGBI_DBG_PDU_TX,
"itt 0x%x, skb 0x%p, len %u/%u, xmit err %d.\n",
task->itt, skb, skb->len, skb->data_len, err);
+
+ kfree_skb(skb);
+
iscsi_conn_printk(KERN_ERR, task->conn, "xmit err %d.\n", err);
iscsi_conn_failure(task->conn, ISCSI_ERR_XMIT_FAILED);
return err;
^ permalink raw reply related
* [PATCH net v5 2/7] cxgb4i: fix credit check for tx_data_wr
From: Karen Xie @ 2014-12-10 1:43 UTC (permalink / raw)
To: linux-scsi, netdev
Cc: kxie, hariprasad, anish, hch, James.Bottomley, michaelc, davem
[PATCH net v5 2/7] cxgb4i: fix credit check for tx_data_wr
From: Karen Xie <kxie@chelsio.com>
make sure any tx credit related checking is done before adding the wr header.
Signed-off-by: Karen Xie <kxie@chelsio.com>
---
drivers/scsi/cxgbi/cxgb4i/cxgb4i.c | 9 +++++----
1 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
index f119a67..56dbd25 100644
--- a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
+++ b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
@@ -547,15 +547,16 @@ static inline void make_tx_data_wr(struct cxgbi_sock *csk, struct sk_buff *skb,
struct fw_ofld_tx_data_wr *req;
unsigned int submode = cxgbi_skcb_ulp_mode(skb) & 3;
unsigned int wr_ulp_mode = 0;
+ bool imm = is_ofld_imm(skb);
req = (struct fw_ofld_tx_data_wr *)__skb_push(skb, sizeof(*req));
- if (is_ofld_imm(skb)) {
+ if (imm) {
req->op_to_immdlen = htonl(FW_WR_OP(FW_OFLD_TX_DATA_WR) |
- FW_WR_COMPL(1) |
- FW_WR_IMMDLEN(dlen));
+ FW_WR_COMPL(1) |
+ FW_WR_IMMDLEN(dlen));
req->flowid_len16 = htonl(FW_WR_FLOWID(csk->tid) |
- FW_WR_LEN16(credits));
+ FW_WR_LEN16(credits));
} else {
req->op_to_immdlen =
cpu_to_be32(FW_WR_OP(FW_OFLD_TX_DATA_WR) |
^ permalink raw reply related
* [PATCH net v5 4/7] cxgb4i: additional types of negative advice
From: Karen Xie @ 2014-12-10 1:43 UTC (permalink / raw)
To: linux-scsi, netdev
Cc: kxie, hariprasad, anish, hch, James.Bottomley, michaelc, davem
[PATCH net v5 4/7] cxgb4i: additional types of negative advice
From: Karen Xie <kxie@chelsio.com>
Treat both CPL_ERR_KEEPALV_NEG_ADVICE and CPL_ERR_PERSIST_NEG_ADVICE as negative advice.
Signed-off-by: Karen Xie <kxie@chelsio.com>
---
drivers/scsi/cxgbi/cxgb4i/cxgb4i.c | 12 +++++++++---
1 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
index 86b9bbc..c4a22e9 100644
--- a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
+++ b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
@@ -846,6 +846,13 @@ static void csk_act_open_retry_timer(unsigned long data)
}
+static inline bool is_neg_adv(unsigned int status)
+{
+ return status == CPL_ERR_RTX_NEG_ADVICE ||
+ status == CPL_ERR_KEEPALV_NEG_ADVICE ||
+ status == CPL_ERR_PERSIST_NEG_ADVICE;
+}
+
static void do_act_open_rpl(struct cxgbi_device *cdev, struct sk_buff *skb)
{
struct cxgbi_sock *csk;
@@ -867,7 +874,7 @@ static void do_act_open_rpl(struct cxgbi_device *cdev, struct sk_buff *skb)
"csk 0x%p,%u,0x%lx. ", (&csk->saddr), (&csk->daddr),
atid, tid, status, csk, csk->state, csk->flags);
- if (status == CPL_ERR_RTX_NEG_ADVICE)
+ if (is_neg_adv(status))
goto rel_skb;
module_put(THIS_MODULE);
@@ -973,8 +980,7 @@ static void do_abort_req_rss(struct cxgbi_device *cdev, struct sk_buff *skb)
(&csk->saddr), (&csk->daddr),
csk, csk->state, csk->flags, csk->tid, req->status);
- if (req->status == CPL_ERR_RTX_NEG_ADVICE ||
- req->status == CPL_ERR_PERSIST_NEG_ADVICE)
+ if (is_neg_adv(req->status))
goto rel_skb;
cxgbi_sock_get(csk);
^ 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