* Re: sctp: deny peeloff operation on asocs with threads sleeping on it
From: Marcelo Ricardo Leitner @ 2017-04-20 13:54 UTC (permalink / raw)
To: Salvatore Bonaccorso, netdev
Cc: Alexander Popov, Ben Hutchings, Xin Long, David S. Miller,
Greg Kroah-Hartman
In-Reply-To: <20170420123257.GA11798@lorien.valinor.li>
Em 20-04-2017 09:32, Salvatore Bonaccorso escreveu:
> Hi
>
> According to the documentation I should have sent this to
> netdev@vger.kernel.org rather than stable@vger.kernel.org.
>
> Rationale: Whilst 00eff2ebbd229758e90659907724c14dd5a18339 went to
> stable, the dfcb9f4f99f1e9a49e43398a7bfbf56927544af1 commit is
> missing.
Cc'ing Greg too.
This is probably because I noticed this commit was missing when Greg
posted the commits being considered for backports, then I raised the
concern and Greg decided to pull it too before David replied.
Not sure if this caused some out-of-the-process stuff.
>
> Full quoting, my original misleaded mail:
>
> On Thu, Apr 20, 2017 at 01:44:05PM +0200, Salvatore Bonaccorso wrote:
>> Hi
>>
>> Apparently the following commit
>>
>> dfcb9f4f99f1e9a49e43398a7bfbf56927544af1 (sctp: deny peeloff operation
>> on asocs with threads sleeping on it)
>>
>> (was not CC'ed stable@vger.kernel.org, but was already applied in
>> 3.2.87 and 3.16.42) is missing from 4.9:
>>
>> 2dcab598484185dea7ec22219c76dcdd59e3cb90 was included in 4.9.11 via
>> 00eff2ebbd229758e90659907724c14dd5a18339 . But
>> dfcb9f4f99f1e9a49e43398a7bfbf56927544af1 is missing from 4.9.
>>
>> This was assigned CVE-2017-6353.
>>
>> Reference:
>> https://marc.info/?l=linux-netdev&m=148785309416337&w=2
>> http://www.openwall.com/lists/oss-security/2017/02/27/2
>>
>> Regards,
>> Salvatore
>
> Regards,
> Salvatore
>
^ permalink raw reply
* [PATCH RFC net-next v4 3/7] sk_buff: remove support for csum_bad in sk_buff
From: Davide Caratti @ 2017-04-20 13:38 UTC (permalink / raw)
To: Tom Herbert, Alexander Duyck, David Laight
Cc: David S . Miller, Marcelo Ricardo Leitner, netdev, linux-sctp
In-Reply-To: <cover.1492692976.git.dcaratti@redhat.com>
This bit was introduced with 5a21232983aa ("net: Support for csum_bad in
skbuff") to reduce the stack workload when processing RX packets carrying
a wrong Internet Checksum. Up to now, only one driver (besides GRO core)
are setting it.
The test on NAPI_GRO_CB(skb)->flush in dev_gro_receive() is now done
before the test on same_flow, to preserve behavior in case of wrong
checksum.
Suggested-by: Tom Herbert <tom@herbertland.com>
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
drivers/net/ethernet/aquantia/atlantic/aq_ring.c | 2 +-
include/linux/netdevice.h | 4 +---
include/linux/skbuff.h | 23 ++---------------------
net/bridge/netfilter/nft_reject_bridge.c | 5 +----
net/core/dev.c | 8 +++-----
net/ipv4/netfilter/nf_reject_ipv4.c | 2 +-
net/ipv6/netfilter/nf_reject_ipv6.c | 3 ---
7 files changed, 9 insertions(+), 38 deletions(-)
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
index 3a8a4aa..9a08179 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c
@@ -223,7 +223,7 @@ int aq_ring_rx_clean(struct aq_ring_s *self, int *work_done, int budget)
skb->protocol = eth_type_trans(skb, ndev);
if (unlikely(buff->is_cso_err)) {
++self->stats.rx.errors;
- __skb_mark_checksum_bad(skb);
+ skb->ip_summed = CHECKSUM_NONE;
} else {
if (buff->is_ip_cso) {
__skb_incr_checksum_unnecessary(skb);
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index bf84a67..ab9e3dc 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2546,9 +2546,7 @@ static inline void skb_gro_incr_csum_unnecessary(struct sk_buff *skb)
if (__skb_gro_checksum_validate_needed(skb, zero_okay, check)) \
__ret = __skb_gro_checksum_validate_complete(skb, \
compute_pseudo(skb, proto)); \
- if (__ret) \
- __skb_mark_checksum_bad(skb); \
- else \
+ if (!__ret) \
skb_gro_incr_csum_unnecessary(skb); \
__ret; \
})
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index ec4551b..927309e 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -743,7 +743,7 @@ struct sk_buff {
__u8 csum_valid:1;
__u8 csum_complete_sw:1;
__u8 csum_level:2;
- __u8 csum_bad:1;
+ __u8 __csum_bad_unused:1; /* one bit hole */
__u8 dst_pending_confirm:1;
#ifdef CONFIG_IPV6_NDISC_NODETYPE
@@ -3387,21 +3387,6 @@ static inline void __skb_incr_checksum_unnecessary(struct sk_buff *skb)
}
}
-static inline void __skb_mark_checksum_bad(struct sk_buff *skb)
-{
- /* Mark current checksum as bad (typically called from GRO
- * path). In the case that ip_summed is CHECKSUM_NONE
- * this must be the first checksum encountered in the packet.
- * When ip_summed is CHECKSUM_UNNECESSARY, this is the first
- * checksum after the last one validated. For UDP, a zero
- * checksum can not be marked as bad.
- */
-
- if (skb->ip_summed == CHECKSUM_NONE ||
- skb->ip_summed == CHECKSUM_UNNECESSARY)
- skb->csum_bad = 1;
-}
-
/* Check if we need to perform checksum complete validation.
*
* Returns true if checksum complete is needed, false otherwise
@@ -3455,9 +3440,6 @@ static inline __sum16 __skb_checksum_validate_complete(struct sk_buff *skb,
skb->csum_valid = 1;
return 0;
}
- } else if (skb->csum_bad) {
- /* ip_summed == CHECKSUM_NONE in this case */
- return (__force __sum16)1;
}
skb->csum = psum;
@@ -3517,8 +3499,7 @@ static inline __wsum null_compute_pseudo(struct sk_buff *skb, int proto)
static inline bool __skb_checksum_convert_check(struct sk_buff *skb)
{
- return (skb->ip_summed == CHECKSUM_NONE &&
- skb->csum_valid && !skb->csum_bad);
+ return (skb->ip_summed == CHECKSUM_NONE && skb->csum_valid);
}
static inline void __skb_checksum_convert(struct sk_buff *skb,
diff --git a/net/bridge/netfilter/nft_reject_bridge.c b/net/bridge/netfilter/nft_reject_bridge.c
index 346ef6b..c16dd3a 100644
--- a/net/bridge/netfilter/nft_reject_bridge.c
+++ b/net/bridge/netfilter/nft_reject_bridge.c
@@ -111,7 +111,7 @@ static void nft_reject_br_send_v4_unreach(struct net *net,
__wsum csum;
u8 proto;
- if (oldskb->csum_bad || !nft_bridge_iphdr_validate(oldskb))
+ if (!nft_bridge_iphdr_validate(oldskb))
return;
/* IP header checks: fragment. */
@@ -226,9 +226,6 @@ static bool reject6_br_csum_ok(struct sk_buff *skb, int hook)
__be16 fo;
u8 proto = ip6h->nexthdr;
- if (skb->csum_bad)
- return false;
-
if (skb_csum_unnecessary(skb))
return true;
diff --git a/net/core/dev.c b/net/core/dev.c
index c7aec95..77a2d73 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4533,9 +4533,6 @@ static enum gro_result dev_gro_receive(struct napi_struct *napi, struct sk_buff
if (!(skb->dev->features & NETIF_F_GRO))
goto normal;
- if (skb->csum_bad)
- goto normal;
-
gro_list_prepare(napi, skb);
rcu_read_lock();
@@ -4595,11 +4592,12 @@ static enum gro_result dev_gro_receive(struct napi_struct *napi, struct sk_buff
napi->gro_count--;
}
+ if (NAPI_GRO_CB(skb)->flush)
+ goto normal;
+
if (same_flow)
goto ok;
- if (NAPI_GRO_CB(skb)->flush)
- goto normal;
if (unlikely(napi->gro_count >= MAX_GRO_SKBS)) {
struct sk_buff *nskb = napi->gro_list;
diff --git a/net/ipv4/netfilter/nf_reject_ipv4.c b/net/ipv4/netfilter/nf_reject_ipv4.c
index 7cd8d0d..6f8d9e5 100644
--- a/net/ipv4/netfilter/nf_reject_ipv4.c
+++ b/net/ipv4/netfilter/nf_reject_ipv4.c
@@ -172,7 +172,7 @@ void nf_send_unreach(struct sk_buff *skb_in, int code, int hook)
struct iphdr *iph = ip_hdr(skb_in);
u8 proto;
- if (skb_in->csum_bad || iph->frag_off & htons(IP_OFFSET))
+ if (iph->frag_off & htons(IP_OFFSET))
return;
if (skb_csum_unnecessary(skb_in)) {
diff --git a/net/ipv6/netfilter/nf_reject_ipv6.c b/net/ipv6/netfilter/nf_reject_ipv6.c
index eedee5d..f63b18e 100644
--- a/net/ipv6/netfilter/nf_reject_ipv6.c
+++ b/net/ipv6/netfilter/nf_reject_ipv6.c
@@ -220,9 +220,6 @@ static bool reject6_csum_ok(struct sk_buff *skb, int hook)
__be16 fo;
u8 proto;
- if (skb->csum_bad)
- return false;
-
if (skb_csum_unnecessary(skb))
return true;
--
2.7.4
^ permalink raw reply related
* [PATCH RFC net-next v4 4/7] net: use skb->csum_not_inet to identify packets needing crc32c
From: Davide Caratti @ 2017-04-20 13:38 UTC (permalink / raw)
To: Tom Herbert, Alexander Duyck, David Laight
Cc: David S . Miller, Marcelo Ricardo Leitner, netdev, linux-sctp
In-Reply-To: <cover.1492692976.git.dcaratti@redhat.com>
skb->csum_not_inet carries the indication on which algorithm is needed to
compute checksum on skb in the transmit path, when skb->ip_summed is equal
to CHECKSUM_PARTIAL. If skb carries a SCTP packet and crc32c hasn't been
yet written in L4 header, skb->csum_not_inet is assigned to 1; otherwise,
assume Internet Checksum is needed and thus set skb->csum_not_inet to 0.
Suggested-by: Tom Herbert <tom@herbertland.com>
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
include/linux/skbuff.h | 16 +++++++++-------
net/core/dev.c | 1 +
net/sched/act_csum.c | 1 +
net/sctp/offload.c | 1 +
net/sctp/output.c | 1 +
5 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 927309e..419f4c8 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -189,12 +189,13 @@
*
* NETIF_F_SCTP_CRC - This feature indicates that a device is capable of
* offloading the SCTP CRC in a packet. To perform this offload the stack
- * will set ip_summed to CHECKSUM_PARTIAL and set csum_start and csum_offset
- * accordingly. Note the there is no indication in the skbuff that the
- * CHECKSUM_PARTIAL refers to an SCTP checksum, a driver that supports
- * both IP checksum offload and SCTP CRC offload must verify which offload
- * is configured for a packet presumably by inspecting packet headers; in
- * case, skb_crc32c_csum_help is provided to compute CRC on SCTP packets.
+ * will set set csum_start and csum_offset accordingly, set ip_summed to
+ * CHECKSUM_PARTIAL and set csum_not_inet to 1, to provide an indication in
+ * the skbuff that the CHECKSUM_PARTIAL refers to CRC32c.
+ * A driver that supports both IP checksum offload and SCTP CRC32c offload
+ * must verify which offload is configured for a packet by testing the
+ * value of skb->csum_not_inet; skb_crc32c_csum_help is provided to resolve
+ * CHECKSUM_PARTIAL on skbs where csum_not_inet is set to 1.
*
* NETIF_F_FCOE_CRC - This feature indicates that a device is capable of
* offloading the FCOE CRC in a packet. To perform this offload the stack
@@ -615,6 +616,7 @@ static inline bool skb_mstamp_after(const struct skb_mstamp *t1,
* @wifi_acked_valid: wifi_acked was set
* @wifi_acked: whether frame was acked on wifi or not
* @no_fcs: Request NIC to treat last 4 bytes as Ethernet FCS
+ * @csum_not_inet: use CRC32c to resolve CHECKSUM_PARTIAL
* @dst_pending_confirm: need to confirm neighbour
* @napi_id: id of the NAPI struct this skb came from
* @secmark: security marking
@@ -743,7 +745,7 @@ struct sk_buff {
__u8 csum_valid:1;
__u8 csum_complete_sw:1;
__u8 csum_level:2;
- __u8 __csum_bad_unused:1; /* one bit hole */
+ __u8 csum_not_inet:1;
__u8 dst_pending_confirm:1;
#ifdef CONFIG_IPV6_NDISC_NODETYPE
diff --git a/net/core/dev.c b/net/core/dev.c
index 77a2d73..9f56f87 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2642,6 +2642,7 @@ int skb_crc32c_csum_help(struct sk_buff *skb)
}
*(__le32 *)(skb->data + offset) = crc32c_csum;
skb->ip_summed = CHECKSUM_NONE;
+ skb->csum_not_inet = 0;
out:
return ret;
}
diff --git a/net/sched/act_csum.c b/net/sched/act_csum.c
index ab6fdbd..3317a2f 100644
--- a/net/sched/act_csum.c
+++ b/net/sched/act_csum.c
@@ -350,6 +350,7 @@ static int tcf_csum_sctp(struct sk_buff *skb, unsigned int ihl,
sctph->checksum = sctp_compute_cksum(skb,
skb_network_offset(skb) + ihl);
skb->ip_summed = CHECKSUM_NONE;
+ skb->csum_not_inet = 0;
return 1;
}
diff --git a/net/sctp/offload.c b/net/sctp/offload.c
index 378f462..ef156ac 100644
--- a/net/sctp/offload.c
+++ b/net/sctp/offload.c
@@ -35,6 +35,7 @@
static __le32 sctp_gso_make_checksum(struct sk_buff *skb)
{
skb->ip_summed = CHECKSUM_NONE;
+ skb->csum_not_inet = 0;
return sctp_compute_cksum(skb, skb_transport_offset(skb));
}
diff --git a/net/sctp/output.c b/net/sctp/output.c
index 1409a87..e2edf2e 100644
--- a/net/sctp/output.c
+++ b/net/sctp/output.c
@@ -538,6 +538,7 @@ static int sctp_packet_pack(struct sctp_packet *packet,
} else {
chksum:
head->ip_summed = CHECKSUM_PARTIAL;
+ head->csum_not_inet = 1;
head->csum_start = skb_transport_header(head) - head->head;
head->csum_offset = offsetof(struct sctphdr, checksum);
}
--
2.7.4
^ permalink raw reply related
* [PATCH RFC net-next v4 7/7] sk_buff.h: improve description of CHECKSUM_{COMPLETE,UNNECESSARY}
From: Davide Caratti @ 2017-04-20 13:38 UTC (permalink / raw)
To: Tom Herbert, Alexander Duyck, David Laight
Cc: David S . Miller, Marcelo Ricardo Leitner, netdev, linux-sctp
In-Reply-To: <cover.1492692976.git.dcaratti@redhat.com>
Add FCoE to the list of protocols that can set CHECKSUM_UNNECESSARY; add a
note to CHECKSUM_COMPLETE section to specify that it does not apply to SCTP
and FCoE protocols.
Suggested-by: Tom Herbert <tom@herbertland.com>
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
include/linux/skbuff.h | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 4002c11..c902b77 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -109,6 +109,7 @@
* may perform further validation in this case.
* GRE: only if the checksum is present in the header.
* SCTP: indicates the CRC in SCTP header has been validated.
+ * FCOE: indicates the CRC in FC frame has been validated.
*
* skb->csum_level indicates the number of consecutive checksums found in
* the packet minus one that have been verified as CHECKSUM_UNNECESSARY.
@@ -126,8 +127,10 @@
* packet as seen by netif_rx() and fills out in skb->csum. Meaning, the
* hardware doesn't need to parse L3/L4 headers to implement this.
*
- * Note: Even if device supports only some protocols, but is able to produce
- * skb->csum, it MUST use CHECKSUM_COMPLETE, not CHECKSUM_UNNECESSARY.
+ * Notes:
+ * - Even if device supports only some protocols, but is able to produce
+ * skb->csum, it MUST use CHECKSUM_COMPLETE, not CHECKSUM_UNNECESSARY.
+ * - CHECKSUM_COMPLETE is not applicable to SCTP and FCoE protocols.
*
* CHECKSUM_PARTIAL:
*
--
2.7.4
^ permalink raw reply related
* [PATCH RFC net-next v4 6/7] openvswitch: more accurate checksumming in queue_userspace_packet()
From: Davide Caratti @ 2017-04-20 13:38 UTC (permalink / raw)
To: Tom Herbert, Alexander Duyck, David Laight
Cc: David S . Miller, Marcelo Ricardo Leitner, netdev, linux-sctp
In-Reply-To: <cover.1492692976.git.dcaratti@redhat.com>
if skb carries an SCTP packet and ip_summed is CHECKSUM_PARTIAL, it needs
CRC32c in place of Internet Checksum: use skb_csum_hwoffload_help to avoid
corrupting such packets while queueing them towards userspace.
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
net/openvswitch/datapath.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index 7b17da9..9ddc9f8 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -453,7 +453,7 @@ static int queue_userspace_packet(struct datapath *dp, struct sk_buff *skb,
/* Complete checksum if needed */
if (skb->ip_summed == CHECKSUM_PARTIAL &&
- (err = skb_checksum_help(skb)))
+ (err = skb_csum_hwoffload_help(skb, 0)))
goto out;
/* Older versions of OVS user space enforce alignment of the last
--
2.7.4
^ permalink raw reply related
* [PATCH RFC net-next v4 5/7] net: more accurate checksumming in validate_xmit_skb()
From: Davide Caratti @ 2017-04-20 13:38 UTC (permalink / raw)
To: Tom Herbert, Alexander Duyck, David Laight
Cc: David S . Miller, Marcelo Ricardo Leitner, netdev, linux-sctp
In-Reply-To: <cover.1492692976.git.dcaratti@redhat.com>
skb_csum_hwoffload_help() uses netdev features and skb->csum_not_inet to
determine if skb needs software computation of Internet Checksum or crc32c
(or nothing, if this computation can be done by the hardware). Use it in
place of skb_checksum_help() in validate_xmit_skb() to avoid corruption
of non-GSO SCTP packets having skb->ip_summed equal to CHECKSUM_PARTIAL.
While at it, remove references to skb_csum_off_chk* functions, since they
are not present anymore in Linux since commit cf53b1da73bd ('Revert "net:
Add driver helper functions to determine checksum"').
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
Documentation/networking/checksum-offloads.txt | 11 +++++++----
include/linux/netdevice.h | 3 +++
include/linux/skbuff.h | 13 +++++--------
net/core/dev.c | 14 ++++++++++++--
4 files changed, 27 insertions(+), 14 deletions(-)
diff --git a/Documentation/networking/checksum-offloads.txt b/Documentation/networking/checksum-offloads.txt
index 56e3686..d52d191 100644
--- a/Documentation/networking/checksum-offloads.txt
+++ b/Documentation/networking/checksum-offloads.txt
@@ -35,6 +35,9 @@ This interface only allows a single checksum to be offloaded. Where
encapsulation is used, the packet may have multiple checksum fields in
different header layers, and the rest will have to be handled by another
mechanism such as LCO or RCO.
+CRC32c can also be offloaded using this interface, by means of filling
+ skb->csum_start and skb->csum_offset as described above, and setting
+ skb->csum_not_inet: see skbuff.h comment (section 'D') for more details.
No offloading of the IP header checksum is performed; it is always done in
software. This is OK because when we build the IP header, we obviously
have it in cache, so summing it isn't expensive. It's also rather short.
@@ -49,9 +52,9 @@ A driver declares its offload capabilities in netdev->hw_features; see
and csum_offset given in the SKB; if it tries to deduce these itself in
hardware (as some NICs do) the driver should check that the values in the
SKB match those which the hardware will deduce, and if not, fall back to
- checksumming in software instead (with skb_checksum_help or one of the
- skb_csum_off_chk* functions as mentioned in include/linux/skbuff.h). This
- is a pain, but that's what you get when hardware tries to be clever.
+ checksumming in software instead (with skb_csum_hwoffload_help() or one of
+ the skb_checksum_help() / skb_crc32c_csum_help functions, as mentioned in
+ include/linux/skbuff.h).
The stack should, for the most part, assume that checksum offload is
supported by the underlying device. The only place that should check is
@@ -60,7 +63,7 @@ The stack should, for the most part, assume that checksum offload is
may include other offloads besides TX Checksum Offload) and, if they are
not supported or enabled on the device (determined by netdev->features),
performs the corresponding offload in software. In the case of TX
- Checksum Offload, that means calling skb_checksum_help(skb).
+ Checksum Offload, that means calling skb_csum_hwoffload_help(skb, features).
LCO: Local Checksum Offload
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index ab9e3dc..45e8958 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3897,6 +3897,9 @@ void netdev_rss_key_fill(void *buffer, size_t len);
int dev_get_nest_level(struct net_device *dev);
int skb_checksum_help(struct sk_buff *skb);
int skb_crc32c_csum_help(struct sk_buff *skb);
+int skb_csum_hwoffload_help(struct sk_buff *skb,
+ const netdev_features_t features);
+
struct sk_buff *__skb_gso_segment(struct sk_buff *skb,
netdev_features_t features, bool tx_path);
struct sk_buff *skb_mac_gso_segment(struct sk_buff *skb,
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 419f4c8..4002c11 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -162,14 +162,11 @@
*
* NETIF_F_IP_CSUM and NETIF_F_IPV6_CSUM are being deprecated in favor of
* NETIF_F_HW_CSUM. New devices should use NETIF_F_HW_CSUM to indicate
- * checksum offload capability. If a device has limited checksum capabilities
- * (for instance can only perform NETIF_F_IP_CSUM or NETIF_F_IPV6_CSUM as
- * described above) a helper function can be called to resolve
- * CHECKSUM_PARTIAL. The helper functions are skb_csum_off_chk*. The helper
- * function takes a spec argument that describes the protocol layer that is
- * supported for checksum offload and can be called for each packet. If a
- * packet does not match the specification for offload, skb_checksum_help
- * is called to resolve the checksum.
+ * checksum offload capability.
+ * skb_csum_hwoffload_help() can be called to resolve CHECKSUM_PARTIAL based
+ * on network device checksumming capabilities: if a packet does not match
+ * them, skb_checksum_help or skb_crc32c_help (depending on the value of
+ * csum_not_inet, see item D.) is called to resolve the checksum.
*
* CHECKSUM_NONE:
*
diff --git a/net/core/dev.c b/net/core/dev.c
index 9f56f87..440ace0 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2989,6 +2989,17 @@ static struct sk_buff *validate_xmit_vlan(struct sk_buff *skb,
return skb;
}
+int skb_csum_hwoffload_help(struct sk_buff *skb,
+ const netdev_features_t features)
+{
+ if (unlikely(skb->csum_not_inet))
+ return !!(features & NETIF_F_SCTP_CRC) ? 0 :
+ skb_crc32c_csum_help(skb);
+
+ return !!(features & NETIF_F_CSUM_MASK) ? 0 : skb_checksum_help(skb);
+}
+EXPORT_SYMBOL(skb_csum_hwoffload_help);
+
static struct sk_buff *validate_xmit_skb(struct sk_buff *skb, struct net_device *dev)
{
netdev_features_t features;
@@ -3024,8 +3035,7 @@ static struct sk_buff *validate_xmit_skb(struct sk_buff *skb, struct net_device
else
skb_set_transport_header(skb,
skb_checksum_start_offset(skb));
- if (!(features & NETIF_F_CSUM_MASK) &&
- skb_checksum_help(skb))
+ if (skb_csum_hwoffload_help(skb, features))
goto out_kfree_skb;
}
}
--
2.7.4
^ permalink raw reply related
* [PATCH RFC net-next v4 2/7] net: introduce skb_crc32c_csum_help
From: Davide Caratti @ 2017-04-20 13:38 UTC (permalink / raw)
To: Tom Herbert, Alexander Duyck, David Laight
Cc: David S . Miller, Marcelo Ricardo Leitner, netdev, linux-sctp
In-Reply-To: <cover.1492692976.git.dcaratti@redhat.com>
skb_crc32c_csum_help is like skb_checksum_help, but it is designed for
checksumming SCTP packets using crc32c (see RFC3309), provided that
libcrc32c.ko has been loaded before. In case libcrc32c is not loaded,
invoking skb_crc32c_csum_help on a skb results in one the following
printouts:
warn_crc32c_csum_update: attempt to compute crc32c without libcrc32c.ko
warn_crc32c_csum_combine: attempt to compute crc32c without libcrc32c.ko
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
include/linux/netdevice.h | 1 +
include/linux/skbuff.h | 3 ++-
net/core/dev.c | 40 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 43 insertions(+), 1 deletion(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index b0aa089..bf84a67 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3898,6 +3898,7 @@ void netdev_rss_key_fill(void *buffer, size_t len);
int dev_get_nest_level(struct net_device *dev);
int skb_checksum_help(struct sk_buff *skb);
+int skb_crc32c_csum_help(struct sk_buff *skb);
struct sk_buff *__skb_gso_segment(struct sk_buff *skb,
netdev_features_t features, bool tx_path);
struct sk_buff *skb_mac_gso_segment(struct sk_buff *skb,
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index ba3ae21..ec4551b 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -193,7 +193,8 @@
* accordingly. Note the there is no indication in the skbuff that the
* CHECKSUM_PARTIAL refers to an SCTP checksum, a driver that supports
* both IP checksum offload and SCTP CRC offload must verify which offload
- * is configured for a packet presumably by inspecting packet headers.
+ * is configured for a packet presumably by inspecting packet headers; in
+ * case, skb_crc32c_csum_help is provided to compute CRC on SCTP packets.
*
* NETIF_F_FCOE_CRC - This feature indicates that a device is capable of
* offloading the FCOE CRC in a packet. To perform this offload the stack
diff --git a/net/core/dev.c b/net/core/dev.c
index 5d33e2b..c7aec95 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -140,6 +140,7 @@
#include <linux/hrtimer.h>
#include <linux/netfilter_ingress.h>
#include <linux/crash_dump.h>
+#include <linux/sctp.h>
#include "net-sysfs.h"
@@ -2606,6 +2607,45 @@ int skb_checksum_help(struct sk_buff *skb)
}
EXPORT_SYMBOL(skb_checksum_help);
+int skb_crc32c_csum_help(struct sk_buff *skb)
+{
+ __le32 crc32c_csum;
+ int ret = 0, offset;
+
+ if (skb->ip_summed != CHECKSUM_PARTIAL)
+ goto out;
+
+ if (unlikely(skb_is_gso(skb)))
+ goto out;
+
+ /* Before computing a checksum, we should make sure no frag could
+ * be modified by an external entity : checksum could be wrong.
+ */
+ if (unlikely(skb_has_shared_frag(skb))) {
+ ret = __skb_linearize(skb);
+ if (ret)
+ goto out;
+ }
+
+ offset = skb_checksum_start_offset(skb);
+ crc32c_csum = cpu_to_le32(~__skb_checksum(skb, offset,
+ skb->len - offset, ~(__u32)0,
+ crc32c_csum_stub));
+ offset += offsetof(struct sctphdr, checksum);
+ BUG_ON(offset >= skb_headlen(skb));
+
+ if (skb_cloned(skb) &&
+ !skb_clone_writable(skb, offset + sizeof(__le32))) {
+ ret = pskb_expand_head(skb, 0, 0, GFP_ATOMIC);
+ if (ret)
+ goto out;
+ }
+ *(__le32 *)(skb->data + offset) = crc32c_csum;
+ skb->ip_summed = CHECKSUM_NONE;
+out:
+ return ret;
+}
+
__be16 skb_network_protocol(struct sk_buff *skb, int *depth)
{
__be16 type = skb->protocol;
--
2.7.4
^ permalink raw reply related
* [PATCH RFC net-next v4 1/7] skbuff: add stub to help computing crc32c on SCTP packets
From: Davide Caratti @ 2017-04-20 13:38 UTC (permalink / raw)
To: Tom Herbert, Alexander Duyck, David Laight
Cc: David S . Miller, Marcelo Ricardo Leitner, netdev, linux-sctp
In-Reply-To: <cover.1492692976.git.dcaratti@redhat.com>
sctp_compute_checksum requires crc32c symbol (provided by libcrc32c), so
it can't be used in net core. Like it has been done previously with other
symbols (e.g. ipv6_dst_lookup), introduce a stub struct skb_checksum_ops
to allow computation of crc32c checksum in net core after sctp.ko (and thus
libcrc32c) has been loaded.
Reviewed-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
include/linux/skbuff.h | 2 ++
net/core/skbuff.c | 24 ++++++++++++++++++++++++
net/sctp/offload.c | 7 +++++++
3 files changed, 33 insertions(+)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 741d75c..ba3ae21 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -3127,6 +3127,8 @@ struct skb_checksum_ops {
__wsum (*combine)(__wsum csum, __wsum csum2, int offset, int len);
};
+extern const struct skb_checksum_ops *crc32c_csum_stub __read_mostly;
+
__wsum __skb_checksum(const struct sk_buff *skb, int offset, int len,
__wsum csum, const struct skb_checksum_ops *ops);
__wsum skb_checksum(const struct sk_buff *skb, int offset, int len,
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index ad2af56..182608b 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -2242,6 +2242,30 @@ __wsum skb_copy_and_csum_bits(const struct sk_buff *skb, int offset,
}
EXPORT_SYMBOL(skb_copy_and_csum_bits);
+static __wsum warn_crc32c_csum_update(const void *buff, int len, __wsum sum)
+{
+ net_warn_ratelimited(
+ "%s: attempt to compute crc32c without libcrc32c.ko\n",
+ __func__);
+ return 0;
+}
+
+static __wsum warn_crc32c_csum_combine(__wsum csum, __wsum csum2,
+ int offset, int len)
+{
+ net_warn_ratelimited(
+ "%s: attempt to compute crc32c without libcrc32c.ko\n",
+ __func__);
+ return 0;
+}
+
+const struct skb_checksum_ops *crc32c_csum_stub __read_mostly =
+ &(struct skb_checksum_ops) {
+ .update = warn_crc32c_csum_update,
+ .combine = warn_crc32c_csum_combine,
+};
+EXPORT_SYMBOL(crc32c_csum_stub);
+
/**
* skb_zerocopy_headlen - Calculate headroom needed for skb_zerocopy()
* @from: source buffer
diff --git a/net/sctp/offload.c b/net/sctp/offload.c
index 4f5a2b5..378f462 100644
--- a/net/sctp/offload.c
+++ b/net/sctp/offload.c
@@ -98,6 +98,12 @@ static const struct net_offload sctp6_offload = {
},
};
+static const struct skb_checksum_ops *crc32c_csum_ops __read_mostly =
+ &(struct skb_checksum_ops) {
+ .update = sctp_csum_update,
+ .combine = sctp_csum_combine,
+};
+
int __init sctp_offload_init(void)
{
int ret;
@@ -110,6 +116,7 @@ int __init sctp_offload_init(void)
if (ret)
goto ipv4;
+ crc32c_csum_stub = crc32c_csum_ops;
return ret;
ipv4:
--
2.7.4
^ permalink raw reply related
* [PATCH RFC net-next v4 0/7] net: improve support for SCTP checksums
From: Davide Caratti @ 2017-04-20 13:38 UTC (permalink / raw)
To: Tom Herbert, Alexander Duyck, David Laight
Cc: David S . Miller, Marcelo Ricardo Leitner, netdev, linux-sctp
In-Reply-To: <CALx6S36rem=OuN_At6qYA=se5cpuYM1N2R8efoaszvo8b8Tz5A@mail.gmail.com>
hello Tom,
On Fri, 2017-04-07 at 11:11 -0700, Tom Herbert wrote:
> maybe just call it csum_not_ip then. Then just do "if
> (unlikely(skb->csum_not_ip)) ..."
Ok, done. V4 uses this bit for SCTP only and leaves unmodified behavior
when offloaded FCoE frames are processed. Further work is still possible
to extend this fix for FCoE, if needed, either by using additional sk_buff
bits, or using skb->csum_not_ip and use other data (e.g. skb->csum_offset)
to distinguish SCTP from FCoE.
> the only case where this new bit is relevant is when
> CHECKSUM_PARTIAL for a CRC is being done. When it's set for offloading
> sctp crc it must be set. When CRC is resolved, in the helper for
> instance, it must be cleared.
in V4 the bit is set when SCTP packets with offloaded checksum are
generated; the bit is cleared when CRC32c is resolved for such packets
(i.e. skb->ip_summed transitions from CHECKSUM_PARTIAL to CHECKSUM_NONE).
Any feedbacks are appreciated!
thank you in advance,
--
davide
Davide Caratti (7):
skbuff: add stub to help computing crc32c on SCTP packets
net: introduce skb_crc32c_csum_help
sk_buff: remove support for csum_bad in sk_buff
net: use skb->csum_not_inet to identify packets needing crc32c
net: more accurate checksumming in validate_xmit_skb()
openvswitch: more accurate checksumming in queue_userspace_packet()
sk_buff.h: improve description of CHECKSUM_{COMPLETE,UNNECESSARY}
Documentation/networking/checksum-offloads.txt | 11 +++--
drivers/net/ethernet/aquantia/atlantic/aq_ring.c | 2 +-
include/linux/netdevice.h | 8 +--
include/linux/skbuff.h | 58 +++++++++-------------
net/bridge/netfilter/nft_reject_bridge.c | 5 +-
net/core/dev.c | 63 +++++++++++++++++++++---
net/core/skbuff.c | 24 +++++++++
net/ipv4/netfilter/nf_reject_ipv4.c | 2 +-
net/ipv6/netfilter/nf_reject_ipv6.c | 3 --
net/openvswitch/datapath.c | 2 +-
net/sched/act_csum.c | 1 +
net/sctp/offload.c | 8 +++
net/sctp/output.c | 1 +
13 files changed, 128 insertions(+), 60 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH v2] net: arc_emac: switch to phy_start()/phy_stop()
From: Alexander Kochetkov @ 2017-04-20 13:29 UTC (permalink / raw)
To: David S. Miller, Philippe Reynes, Peter Chen, Wei Yongjun, netdev,
linux-kernel
Cc: Alexander Kochetkov
Currently driver use phy_start_aneg() in arc_emac_open() to bring
up PHY. But phy_start() function is more appropriate for this purposes.
Besides that it call phy_start_aneg() as part of PHY startup sequence
it also can correctly bring up PHY from error and suspended states.
So the patch replace phy_start_aneg() to phy_start().
Also the patch add call to phy_stop() to arc_emac_stop() to allow
the PHY device to be fully suspended when the interface is unused.
Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com>
---
Changes in v2:
- Updated commit message to clarify changes
drivers/net/ethernet/arc/emac_main.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/arc/emac_main.c b/drivers/net/ethernet/arc/emac_main.c
index abc9f2a..188676d 100644
--- a/drivers/net/ethernet/arc/emac_main.c
+++ b/drivers/net/ethernet/arc/emac_main.c
@@ -434,7 +434,7 @@ static int arc_emac_open(struct net_device *ndev)
/* Enable EMAC */
arc_reg_or(priv, R_CTRL, EN_MASK);
- phy_start_aneg(ndev->phydev);
+ phy_start(ndev->phydev);
netif_start_queue(ndev);
@@ -556,6 +556,8 @@ static int arc_emac_stop(struct net_device *ndev)
napi_disable(&priv->napi);
netif_stop_queue(ndev);
+ phy_stop(ndev->phydev);
+
/* Disable interrupts */
arc_reg_clr(priv, R_ENABLE, RXINT_MASK | TXINT_MASK | ERR_MASK);
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH net-next v4 1/2] net sched actions: dump more than TCA_ACT_MAX_PRIO actions per batch
From: Jamal Hadi Salim @ 2017-04-20 13:27 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Jiri Pirko, davem, netdev, xiyou.wangcong
In-Reply-To: <1492690694.22296.32.camel@edumazet-glaptop3.roam.corp.google.com>
On 17-04-20 08:18 AM, Eric Dumazet wrote:
> On Thu, 2017-04-20 at 06:42 -0400, Jamal Hadi Salim wrote:
>
>>
>> They are not the same issue Jiri. We have used bitmasks fine on netlink
>> message for a millenia. Nobody sets garbage on a bitmask they are not
>> supposed to touch. The struct padding thing is a shame the way it
>> turned out - now netlink can no longer have a claim to be a (good)
>> wire protocol.
>
> Except that users wrote programs, and these programs work today.
>
> By changing the kernel and recognizing new flags in existing padding,
> you might break the programs.
>
> This is not acceptable. Period.
>
> Had we checked the padding being 0 in old kernels, this change would
> have been possible today.
>
> But because old kernels did not care of the padding contents, then there
> is no way new kernel can suddenly trust them at all.
>
> Please Jamal, you have to forget this nonsense.
That is fine. We can rule out netlink ever being able to work
across machines. That was the dream in the past. Lets close that
discussion.
The issue Jiri is bringing up is unrelated. He is talking about
a bitmap and conflating it with a data structure. They are not
the same issue.
cheers,
jamal
>
^ permalink raw reply
* Re: Heads-up: two regressions in v4.11-rc series
From: Frederic Weisbecker @ 2017-04-20 13:25 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Linus Torvalds, Andrew Morton, Mel Gorman, Tariq Toukan, LKML,
linux-mm, netdev@vger.kernel.org, peterz
In-Reply-To: <20170420110042.73d01e0f@redhat.com>
On Thu, Apr 20, 2017 at 11:00:42AM +0200, Jesper Dangaard Brouer wrote:
> Hi Linus,
>
> Just wanted to give a heads-up on two regressions in 4.11-rc series.
>
> (1) page allocator optimization revert
>
> Mel Gorman and I have been playing with optimizing the page allocator,
> but Tariq spotted that we caused a regression for (NIC) drivers that
> refill DMA RX rings in softirq context.
>
> The end result was a revert, and this is waiting in AKPMs quilt queue:
> http://ozlabs.org/~akpm/mmots/broken-out/revert-mm-page_alloc-only-use-per-cpu-allocator-for-irq-safe-requests.patch
>
>
> (2) Busy softirq can cause userspace not to be scheduled
>
> I bisected the problem to a499a5a14dbd ("sched/cputime: Increment
> kcpustat directly on irqtime account"). See email thread with
> Subject: Bisected softirq accounting issue in v4.11-rc1~170^2~28
> http://lkml.kernel.org/r/20170328101403.34a82fbf@redhat.com
>
> I don't know the scheduler code well enough to fix this, and will have
> to rely others to figure out this scheduler regression.
>
> To make it clear: I'm only seeing this scheduler regression when a
> remote host is sending many many network packets, towards the kernel
> which keeps NAPI/softirq busy all the time. A possible hint: tool
> "top" only shows this in "si" column, while on v4.10 "top" also blames
> "ksoftirqd/N", plus "ps" reported cputime (0:00) seems wrong for ksoftirqd.
(I'm currently working on reproducing that one.)
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [RFC PATCH net] net/mlx5e: Race between mlx5e_update_stats() and getting the stats
From: Saeed Mahameed @ 2017-04-20 13:22 UTC (permalink / raw)
To: Eric Dumazet
Cc: Martin KaFai Lau, Linux Netdev List, Saeed Mahameed, Kernel Team
In-Reply-To: <1492644938.22296.19.camel@edumazet-glaptop3.roam.corp.google.com>
On Thu, Apr 20, 2017 at 2:35 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Wed, 2017-04-19 at 14:53 -0700, Martin KaFai Lau wrote:
>
>> Right, a temp and a memcpy should be enough to solve our spike problem.
>> It may be the right fix for net.
>>
>> Agree that using a spinlock is better (likely changing state_lock
>> to spinlock). A quick grep shows 80 line changes. Saeed, thoughts?
No, changing the current state_lock is a bad idea, as Eric said it is
for a reason,
to synchronize between arbitrary ring/device state changes which might sleep.
memcpy is a good idea to better hide or delay the issue :),
new dedicated spin lock is the right way to go, As Eric suggested below.
BTW, very nice catch Martin, I just got back from vacation to work on this bug,
and you already root caused it, Thanks !!
>
> I was not advising replacing the mutex (maybe it is a mutex for good
> reason), I simply suggested to use another spinlock only for this very
> specific section.
>
> Something like :
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h
> index dc52053128bc752ccd398449330c24c0bdf8b3a1..9b2e1b79fded22d55e9409cb572308190679cfdd 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
> @@ -722,6 +722,7 @@ struct mlx5e_priv {
> struct mlx5_core_dev *mdev;
> struct net_device *netdev;
> struct mlx5e_stats stats;
> + spinlock_t stats_lock;
> struct mlx5e_tstamp tstamp;
> u16 q_counter;
> #ifdef CONFIG_MLX5_CORE_EN_DCB
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
> index a004a5a1a4c22a742ef3f9939769c6b5c9445f46..b4b7d43bf899cadca2c2a17151d35acac9773859 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
> @@ -315,9 +315,11 @@ static void mlx5e_get_ethtool_stats(struct net_device *dev,
> mlx5e_update_stats(priv);
> mutex_unlock(&priv->state_lock);
>
> + spin_lock(&priv->stats_lock);
> for (i = 0; i < NUM_SW_COUNTERS; i++)
> data[idx++] = MLX5E_READ_CTR64_CPU(&priv->stats.sw,
> sw_stats_desc, i);
> + spin_unlock(&priv->stats_lock);
>
> for (i = 0; i < MLX5E_NUM_Q_CNTRS(priv); i++)
> data[idx++] = MLX5E_READ_CTR32_CPU(&priv->stats.qcnt,
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> index 66c133757a5ee8daae122e93322306b1c5c44336..4d6672045b1126a8bab4d6f2035e6a9b830560d2 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> @@ -174,7 +174,7 @@ static void mlx5e_tx_timeout_work(struct work_struct *work)
>
> static void mlx5e_update_sw_counters(struct mlx5e_priv *priv)
> {
> - struct mlx5e_sw_stats *s = &priv->stats.sw;
> + struct mlx5e_sw_stats temp, *s = &temp;
> struct mlx5e_rq_stats *rq_stats;
> struct mlx5e_sq_stats *sq_stats;
> u64 tx_offload_none = 0;
> @@ -229,6 +229,9 @@ static void mlx5e_update_sw_counters(struct mlx5e_priv *priv)
> s->link_down_events_phy = MLX5_GET(ppcnt_reg,
> priv->stats.pport.phy_counters,
> counter_set.phys_layer_cntrs.link_down_events);
> + spin_lock(&priv->stats_lock);
> + memcpy(&priv->stats.sw, s, sizeof(*s));
> + spin_unlock(&priv->stats_lock);
I like this ! minimized the critical section with a temp buffer and a
memcpy .. perfect.
> }
>
> static void mlx5e_update_vport_counters(struct mlx5e_priv *priv)
> @@ -2754,11 +2757,13 @@ mlx5e_get_stats(struct net_device *dev, struct rtnl_link_stats64 *stats)
> stats->tx_packets = PPORT_802_3_GET(pstats, a_frames_transmitted_ok);
> stats->tx_bytes = PPORT_802_3_GET(pstats, a_octets_transmitted_ok);
> } else {
> + spin_lock(&priv->stats_lock);
> stats->rx_packets = sstats->rx_packets;
> stats->rx_bytes = sstats->rx_bytes;
> stats->tx_packets = sstats->tx_packets;
> stats->tx_bytes = sstats->tx_bytes;
> stats->tx_dropped = sstats->tx_queue_dropped;
> + spin_unlock(&priv->stats_lock);
> }
>
> stats->rx_dropped = priv->stats.qcnt.rx_out_of_buffer;
> @@ -3561,6 +3566,8 @@ static void mlx5e_build_nic_netdev_priv(struct mlx5_core_dev *mdev,
>
> mutex_init(&priv->state_lock);
>
> + spin_lock_init(&priv->stats_lock);
> +
> INIT_WORK(&priv->update_carrier_work, mlx5e_update_carrier_work);
> INIT_WORK(&priv->set_rx_mode_work, mlx5e_set_rx_mode_work);
> INIT_WORK(&priv->tx_timeout_work, mlx5e_tx_timeout_work);
>
>
^ permalink raw reply
* raw patch for iproute2 fast action dumping
From: Jamal Hadi Salim @ 2017-04-20 13:18 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev@vger.kernel.org
[-- Attachment #1: Type: text/plain, Size: 46 bytes --]
Not to be used in production.
cheers,
jamal
[-- Attachment #2: act-dump-iprt-patch --]
[-- Type: text/plain, Size: 8314 bytes --]
diff --git a/tc/f_basic.c b/tc/f_basic.c
index d663668..8370ea6 100644
--- a/tc/f_basic.c
+++ b/tc/f_basic.c
@@ -135,7 +135,7 @@ static int basic_print_opt(struct filter_util *qu, FILE *f,
}
if (tb[TCA_BASIC_ACT]) {
- tc_print_action(f, tb[TCA_BASIC_ACT]);
+ tc_print_action(f, tb[TCA_BASIC_ACT], 0);
}
return 0;
diff --git a/tc/f_bpf.c b/tc/f_bpf.c
index df8a259..52027af 100644
--- a/tc/f_bpf.c
+++ b/tc/f_bpf.c
@@ -231,7 +231,7 @@ static int bpf_print_opt(struct filter_util *qu, FILE *f,
}
if (tb[TCA_BPF_ACT])
- tc_print_action(f, tb[TCA_BPF_ACT]);
+ tc_print_action(f, tb[TCA_BPF_ACT], 0);
return 0;
}
diff --git a/tc/f_cgroup.c b/tc/f_cgroup.c
index ecf9909..633700e 100644
--- a/tc/f_cgroup.c
+++ b/tc/f_cgroup.c
@@ -102,7 +102,7 @@ static int cgroup_print_opt(struct filter_util *qu, FILE *f,
}
if (tb[TCA_CGROUP_ACT])
- tc_print_action(f, tb[TCA_CGROUP_ACT]);
+ tc_print_action(f, tb[TCA_CGROUP_ACT], 0);
return 0;
}
diff --git a/tc/f_flow.c b/tc/f_flow.c
index 09ddcaa..b157104 100644
--- a/tc/f_flow.c
+++ b/tc/f_flow.c
@@ -347,7 +347,7 @@ static int flow_print_opt(struct filter_util *fu, FILE *f, struct rtattr *opt,
tc_print_police(f, tb[TCA_FLOW_POLICE]);
if (tb[TCA_FLOW_ACT]) {
fprintf(f, "\n");
- tc_print_action(f, tb[TCA_FLOW_ACT]);
+ tc_print_action(f, tb[TCA_FLOW_ACT], 0);
}
return 0;
}
diff --git a/tc/f_flower.c b/tc/f_flower.c
index 5aac4a0..04733d9 100644
--- a/tc/f_flower.c
+++ b/tc/f_flower.c
@@ -1174,7 +1174,7 @@ static int flower_print_opt(struct filter_util *qu, FILE *f,
}
if (tb[TCA_FLOWER_ACT])
- tc_print_action(f, tb[TCA_FLOWER_ACT]);
+ tc_print_action(f, tb[TCA_FLOWER_ACT], 0);
return 0;
}
diff --git a/tc/f_fw.c b/tc/f_fw.c
index 790bef9..c39789b 100644
--- a/tc/f_fw.c
+++ b/tc/f_fw.c
@@ -160,7 +160,7 @@ static int fw_print_opt(struct filter_util *qu, FILE *f, struct rtattr *opt, __u
if (tb[TCA_FW_ACT]) {
fprintf(f, "\n");
- tc_print_action(f, tb[TCA_FW_ACT]);
+ tc_print_action(f, tb[TCA_FW_ACT], 0);
}
return 0;
}
diff --git a/tc/f_matchall.c b/tc/f_matchall.c
index ac48630..5c8fe2a 100644
--- a/tc/f_matchall.c
+++ b/tc/f_matchall.c
@@ -140,7 +140,7 @@ static int matchall_print_opt(struct filter_util *qu, FILE *f,
}
if (tb[TCA_MATCHALL_ACT])
- tc_print_action(f, tb[TCA_MATCHALL_ACT]);
+ tc_print_action(f, tb[TCA_MATCHALL_ACT], 0);
return 0;
}
diff --git a/tc/f_route.c b/tc/f_route.c
index 30514c4..e88313f 100644
--- a/tc/f_route.c
+++ b/tc/f_route.c
@@ -168,7 +168,7 @@ static int route_print_opt(struct filter_util *qu, FILE *f, struct rtattr *opt,
if (tb[TCA_ROUTE4_POLICE])
tc_print_police(f, tb[TCA_ROUTE4_POLICE]);
if (tb[TCA_ROUTE4_ACT])
- tc_print_action(f, tb[TCA_ROUTE4_ACT]);
+ tc_print_action(f, tb[TCA_ROUTE4_ACT], 0);
return 0;
}
diff --git a/tc/f_rsvp.c b/tc/f_rsvp.c
index 94bfbef..65caeb4 100644
--- a/tc/f_rsvp.c
+++ b/tc/f_rsvp.c
@@ -402,7 +402,7 @@ static int rsvp_print_opt(struct filter_util *qu, FILE *f, struct rtattr *opt, _
}
if (tb[TCA_RSVP_ACT]) {
- tc_print_action(f, tb[TCA_RSVP_ACT]);
+ tc_print_action(f, tb[TCA_RSVP_ACT], 0);
}
if (tb[TCA_RSVP_POLICE])
tc_print_police(f, tb[TCA_RSVP_POLICE]);
diff --git a/tc/f_tcindex.c b/tc/f_tcindex.c
index 784c890..dd1cb47 100644
--- a/tc/f_tcindex.c
+++ b/tc/f_tcindex.c
@@ -173,7 +173,7 @@ static int tcindex_print_opt(struct filter_util *qu, FILE *f,
}
if (tb[TCA_TCINDEX_ACT]) {
fprintf(f, "\n");
- tc_print_action(f, tb[TCA_TCINDEX_ACT]);
+ tc_print_action(f, tb[TCA_TCINDEX_ACT], 0);
}
return 0;
}
diff --git a/tc/f_u32.c b/tc/f_u32.c
index 92c1fcd..dbc6939 100644
--- a/tc/f_u32.c
+++ b/tc/f_u32.c
@@ -1332,7 +1332,7 @@ static int u32_print_opt(struct filter_util *qu, FILE *f, struct rtattr *opt,
}
if (tb[TCA_U32_ACT])
- tc_print_action(f, tb[TCA_U32_ACT]);
+ tc_print_action(f, tb[TCA_U32_ACT], 0);
return 0;
}
diff --git a/tc/m_action.c b/tc/m_action.c
index 6ba7615..c5d390d 100644
--- a/tc/m_action.c
+++ b/tc/m_action.c
@@ -230,7 +230,7 @@ done0:
NEXT_ARG();
slen = strlen(*argv);
if (slen > TC_COOKIE_MAX_SIZE * 2)
- invarg("cookie cannot exceed %d\n",
+ invarg("cookie exceeded len\n",
*argv);
if (hex2mem(*argv, act_ck, slen / 2) < 0)
@@ -341,21 +341,24 @@ tc_print_action_flush(FILE *f, const struct rtattr *arg)
}
int
-tc_print_action(FILE *f, const struct rtattr *arg)
+tc_print_action(FILE *f, const struct rtattr *arg, unsigned short tot_acts)
{
int i;
- struct rtattr *tb[TCA_ACT_MAX_PRIO + 1];
if (arg == NULL)
return 0;
- parse_rtattr_nested(tb, TCA_ACT_MAX_PRIO, arg);
+ if (!tot_acts)
+ tot_acts = TCA_ACT_MAX_PRIO;
+
+ struct rtattr *tb[tot_acts + 1];
+ parse_rtattr_nested(tb, tot_acts, arg);
if (tab_flush && NULL != tb[0] && NULL == tb[1])
return tc_print_action_flush(f, tb[0]);
- for (i = 0; i < TCA_ACT_MAX_PRIO; i++) {
+ for (i = 0; i < tot_acts; i++) {
if (tb[i]) {
fprintf(f, "\n\taction order %d: ", i);
if (tc_print_one_action(f, tb[i]) < 0) {
@@ -375,6 +378,7 @@ int print_action(const struct sockaddr_nl *who,
FILE *fp = (FILE *)arg;
struct tcamsg *t = NLMSG_DATA(n);
int len = n->nlmsg_len;
+ __u32 *tot_acts = NULL;
struct rtattr *tb[TCAA_MAX+1];
len -= NLMSG_LENGTH(sizeof(*t));
@@ -386,6 +390,10 @@ int print_action(const struct sockaddr_nl *who,
parse_rtattr(tb, TCAA_MAX, TA_RTA(t), len);
+ if (tb[TCAA_ACT_COUNT])
+ tot_acts = RTA_DATA(tb[TCAA_ACT_COUNT]);
+
+ fprintf(fp, "total acts %d \n", tot_acts?*tot_acts:0);
if (tb[TCA_ACT_TAB] == NULL) {
if (n->nlmsg_type != RTM_GETACTION)
fprintf(stderr, "print_action: NULL kind\n");
@@ -409,7 +417,9 @@ int print_action(const struct sockaddr_nl *who,
fprintf(fp, "Replaced action ");
}
}
- tc_print_action(fp, tb[TCA_ACT_TAB]);
+
+
+ tc_print_action(fp, tb[TCA_ACT_TAB], tot_acts?*tot_acts:0);
return 0;
}
@@ -422,7 +432,7 @@ static int tc_action_gd(int cmd, unsigned int flags, int *argc_p, char ***argv_p
char **argv = *argv_p;
int prio = 0;
int ret = 0;
- __u32 i;
+ __u32 i = 0;
struct rtattr *tail;
struct rtattr *tail2;
struct nlmsghdr *ans = NULL;
@@ -493,7 +503,8 @@ static int tc_action_gd(int cmd, unsigned int flags, int *argc_p, char ***argv_p
tail2 = NLMSG_TAIL(&req.n);
addattr_l(&req.n, MAX_MSG, ++prio, NULL, 0);
addattr_l(&req.n, MAX_MSG, TCA_ACT_KIND, k, strlen(k) + 1);
- addattr32(&req.n, MAX_MSG, TCA_ACT_INDEX, i);
+ if (i > 0)
+ addattr32(&req.n, MAX_MSG, TCA_ACT_INDEX, i);
tail2->rta_len = (void *) NLMSG_TAIL(&req.n) - (void *) tail2;
}
@@ -560,7 +571,8 @@ static int tc_act_list_or_flush(int argc, char **argv, int event)
{
int ret = 0, prio = 0, msg_size = 0;
char k[16];
- struct rtattr *tail, *tail2;
+ struct rtattr *tail, *tail2, *tail3, *tail4;
+ __u32 msec_since = 120*1000; /* need good way to pass via cli */
struct action_util *a = NULL;
struct {
struct nlmsghdr n;
@@ -597,6 +609,12 @@ static int tc_act_list_or_flush(int argc, char **argv, int event)
tail2->rta_len = (void *) NLMSG_TAIL(&req.n) - (void *) tail2;
tail->rta_len = (void *) NLMSG_TAIL(&req.n) - (void *) tail;
+ tail3 = NLMSG_TAIL(&req.n);
+ addattr32(&req.n, MAX_MSG,/*TCAA_ACT_FLAGS*/2, act_flags);
+ tail3->rta_len = (void *) NLMSG_TAIL(&req.n) - (void *) tail3;
+ tail4 = NLMSG_TAIL(&req.n);
+ addattr32(&req.n, MAX_MSG,/*TCAA_ACT_TIME_FILTER*/4, msec_since);
+ tail4->rta_len = (void *) NLMSG_TAIL(&req.n) - (void *) tail4;
msg_size = NLMSG_ALIGN(req.n.nlmsg_len) - NLMSG_ALIGN(sizeof(struct nlmsghdr));
if (event == RTM_GETACTION) {
diff --git a/tc/tc_util.h b/tc/tc_util.h
index 4db26c6..af578f9 100644
--- a/tc/tc_util.h
+++ b/tc/tc_util.h
@@ -106,7 +106,7 @@ int act_parse_police(struct action_util *a, int *argc_p,
char ***argv_p, int tca_id, struct nlmsghdr *n);
int print_police(struct action_util *a, FILE *f, struct rtattr *tb);
int police_print_xstats(struct action_util *a, FILE *f, struct rtattr *tb);
-int tc_print_action(FILE *f, const struct rtattr *tb);
+int tc_print_action(FILE *f, const struct rtattr *tb, unsigned short tot_acts);
int tc_print_ipt(FILE *f, const struct rtattr *tb);
int parse_action(int *argc_p, char ***argv_p, int tca_id, struct nlmsghdr *n);
void print_tm(FILE *f, const struct tcf_t *tm);
^ permalink raw reply related
* Re: [PATCH v2] mdio_bus: Issue GPIO RESET to PHYs.
From: Andrew Lunn @ 2017-04-20 13:13 UTC (permalink / raw)
To: Roger Quadros
Cc: davem, Florian Fainelli, tony, nsekhar, jsarha, netdev,
linux-omap, linux-kernel
In-Reply-To: <daa29a99-0a17-d316-57eb-3e6162938d8d@ti.com>
On Thu, Apr 20, 2017 at 11:39:20AM +0300, Roger Quadros wrote:
> Some boards [1] leave the PHYs at an invalid state
> during system power-up or reset thus causing unreliability
> issues with the PHY which manifests as PHY not being detected
> or link not functional. To fix this, these PHYs need to be RESET
> via a GPIO connected to the PHY's RESET pin.
>
> Some boards have a single GPIO controlling the PHY RESET pin of all
> PHYs on the bus whereas some others have separate GPIOs controlling
> individual PHY RESETs.
>
> In both cases, the RESET de-assertion cannot be done in the PHY driver
> as the PHY will not probe till its reset is de-asserted.
> So do the RESET de-assertion in the MDIO bus driver.
Hi Roger
Networking patches should include in the subject line which tree they
are for. In this case, net-next. So please make the Subject
[patch v3 net-next] .....
> [1] - am572x-idk, am571x-idk, a437x-idk
>
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
> v2:
> - add device tree binding document (mdio.txt)
> - specify default reset delay in of_mdio.c instead of mdio_bus.c
>
> Documentation/devicetree/bindings/net/mdio.txt | 20 ++++++++++++++++++++
> drivers/net/phy/mdio_bus.c | 22 ++++++++++++++++++++++
> drivers/of/of_mdio.c | 7 +++++++
> include/linux/phy.h | 5 +++++
> 4 files changed, 54 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/mdio.txt
>
> diff --git a/Documentation/devicetree/bindings/net/mdio.txt b/Documentation/devicetree/bindings/net/mdio.txt
> new file mode 100644
> index 0000000..6e703d7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/mdio.txt
> @@ -0,0 +1,20 @@
> +Common MDIO bus properties.
> +
> +These are generic properties that can apply to any MDIO bus.
> +
> +Optional properties:
> +- reset-gpios: List of one or more GPIOs that control the RESET lines
> + of the PHYs on that MDIO bus.
> +- reset-delay-us: RESET pulse width as per PHY datasheet.
It would be good to explicitly say that it is in uS as part of the
comment.
Also, please document that we expect a list of child nodes, one per
device on the bus. These should follow the generic phy.txt, or a
device specific binding document.
> +
> +Example :
> +
It would be good to say something like:
This example shows these optional properties, plus other properties
required for the TI Davinci MDIO driver.
Pointing this out may stop people cut/past the ti,hwmods property.
> + davinci_mdio: ethernet@0x5c030000 {
> + compatible = "ti,davinci_mdio";
> + ti,hwmods = "davinci_mdio";
> + reg = <0x5c030000 0x1000>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reset-gpios = <&gpio2 5 GPIO_ACTIVE_LOW>;
> + reset-delay-us = <2>; /* PHY datasheet states 1uS min */
> + };
And please include at least one PHY on the bus.
Sorry for asking for so much in the documentation. That is the problem
with the documentation being missing to start with.
The code looks good now.
Andrew
^ permalink raw reply
* [PATCH net-next v5 2/2] net sched actions: add time filter for action dumping
From: Jamal Hadi Salim @ 2017-04-20 13:06 UTC (permalink / raw)
To: davem; +Cc: jiri, xiyou.wangcong, eric.dumazet, netdev, Jamal Hadi Salim
In-Reply-To: <1492693582-26810-1-git-send-email-jhs@emojatatu.com>
From: Jamal Hadi Salim <jhs@mojatatu.com>
This adds support for filtering based on time since last used.
When we are dumping a large number of actions it is useful to
have the option of filtering based on when the action was last
used to reduce the amount of data crossing to user space.
With this patch the user space app sets the TCAA_ACT_TIME_DELTA
attribute with the value in milliseconds with "time of interest
since now". The kernel converts this to jiffies and does the
filtering comparison matching entries that have seen activity
since then and returns them to user space.
Old kernels and old tc continue to work in legacy mode since
they dont specify this attribute.
Some example (we have 400 actions bound to 400 filters); at
installation time using hacked tc which sets the time of
interest to 120 seconds:
prompt$ hackedtc actions ls action gact | grep index | wc -l
400
go get some coffee and wait for > 120 seconds and try again:
prompt$ hackedtc actions ls action gact | grep index | wc -l
0
Lets see a filter bound to one of these actions:
..
filter pref 10 u32
filter pref 10 u32 fh 800: ht divisor 1
filter pref 10 u32 fh 800::800 order 2048 key ht 800 bkt 0 flowid 1:10 (rule hit 2 success 1)
match 7f000002/ffffffff at 12 (success 1 )
action order 1: gact action pass
random type none pass val 0
index 23 ref 2 bind 1 installed 1145 sec used 802 sec
Action statistics:
Sent 84 bytes 1 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
....
that coffee took long, no? It was good.
Now lets ping -c 1 127.0.0.2, then run the actions again:
prompt$ hackedtc actions ls action gact | grep index | wc -l
1
More details please:
prompt$ hackedtc -s actions ls action gact
action order 0: gact action pass
random type none pass val 0
index 23 ref 2 bind 1 installed 1270 sec used 30 sec
Action statistics:
Sent 168 bytes 2 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
And the filter?
filter pref 10 u32
filter pref 10 u32 fh 800: ht divisor 1
filter pref 10 u32 fh 800::800 order 2048 key ht 800 bkt 0 flowid 1:10 (rule hit 4 success 2)
match 7f000002/ffffffff at 12 (success 2 )
action order 1: gact action pass
random type none pass val 0
index 23 ref 2 bind 1 installed 1324 sec used 84 sec
Action statistics:
Sent 168 bytes 2 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
include/uapi/linux/rtnetlink.h | 1 +
net/sched/act_api.c | 20 +++++++++++++++++++-
2 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
index d7d28ec..da8a5de 100644
--- a/include/uapi/linux/rtnetlink.h
+++ b/include/uapi/linux/rtnetlink.h
@@ -681,6 +681,7 @@ enum {
#define TCA_ACT_TAB TCAA_ACT_TAB
TCAA_ACT_FLAGS,
TCAA_ACT_COUNT,
+ TCAA_ACT_TIME_DELTA,
__TCAA_MAX,
#define TCAA_MAX (__TCAA_MAX - 1)
};
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index f85b8fd..d163ff4 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -84,6 +84,7 @@ static int tcf_dump_walker(struct tcf_hashinfo *hinfo, struct sk_buff *skb,
{
int err = 0, index = -1, i = 0, s_i = 0, n_i = 0;
unsigned short act_flags = cb->args[2];
+ unsigned long jiffy_since = cb->args[3];
struct nlattr *nest;
spin_lock_bh(&hinfo->lock);
@@ -101,6 +102,11 @@ static int tcf_dump_walker(struct tcf_hashinfo *hinfo, struct sk_buff *skb,
if (index < s_i)
continue;
+ if (jiffy_since &&
+ time_after(jiffy_since,
+ (unsigned long)p->tcfa_tm.lastuse))
+ continue;
+
nest = nla_nest_start(skb, n_i);
if (nest == NULL)
goto nla_put_failure;
@@ -118,9 +124,11 @@ static int tcf_dump_walker(struct tcf_hashinfo *hinfo, struct sk_buff *skb,
}
}
done:
+ if (index > 0)
+ cb->args[0] = index + 1;
+
spin_unlock_bh(&hinfo->lock);
if (n_i) {
- cb->args[0] += n_i;
if (act_flags & ACT_LARGE_DUMP_ON)
cb->args[1] = n_i;
}
@@ -1000,6 +1008,7 @@ static int tcf_action_add(struct net *net, struct nlattr *nla,
static const struct nla_policy tcaa_policy[TCAA_MAX + 1] = {
[TCAA_ACT_FLAGS] = { .type = NLA_U32 },
+ [TCAA_ACT_TIME_DELTA] = { .type = NLA_U32 },
};
static int tc_ctl_action(struct sk_buff *skb, struct nlmsghdr *n,
@@ -1091,6 +1100,8 @@ static int tc_dump_action(struct sk_buff *skb, struct netlink_callback *cb)
struct nlattr *kind = NULL;
struct nlattr *count_attr = NULL;
u32 act_flags = 0;
+ u32 msecs_since = 0;
+ unsigned long jiffy_since = 0;
ret = nlmsg_parse(cb->nlh, sizeof(struct tcamsg), tcaa, TCAA_MAX,
tcaa_policy, NULL);
@@ -1111,12 +1122,19 @@ static int tc_dump_action(struct sk_buff *skb, struct netlink_callback *cb)
if (tcaa[TCAA_ACT_FLAGS])
act_flags = nla_get_u32(tcaa[TCAA_ACT_FLAGS]);
+ if (tcaa[TCAA_ACT_TIME_DELTA])
+ msecs_since = nla_get_u32(tcaa[TCAA_ACT_TIME_DELTA]);
+
nlh = nlmsg_put(skb, NETLINK_CB(cb->skb).portid, cb->nlh->nlmsg_seq,
cb->nlh->nlmsg_type, sizeof(*t), 0);
if (!nlh)
goto out_module_put;
+ if (msecs_since)
+ jiffy_since = jiffies - msecs_to_jiffies(msecs_since);
+
cb->args[2] = act_flags;
+ cb->args[3] = jiffy_since;
t = nlmsg_data(nlh);
t->tca_family = AF_UNSPEC;
--
1.9.1
^ permalink raw reply related
* [PATCH net-next v5 1/2] net sched actions: dump more than TCA_ACT_MAX_PRIO actions per batch
From: Jamal Hadi Salim @ 2017-04-20 13:06 UTC (permalink / raw)
To: davem; +Cc: jiri, xiyou.wangcong, eric.dumazet, netdev, Jamal Hadi Salim
In-Reply-To: <1492693582-26810-1-git-send-email-jhs@emojatatu.com>
From: Jamal Hadi Salim <jhs@mojatatu.com>
When you dump hundreds of thousands of actions, getting only 32 per
dump batch even when the socket buffer and memory allocations allow
is inefficient.
With this change, the user will get as many as possibly fitting
within the given constraints available to the kernel.
A new top level TLV space is introduced. An attribute
TCAA_ACT_FLAGS is used to carry the flags indicating the user
is capable of processing these large dumps. Older user space which
doesn't set this flag doesn't get the large (than 32) batches.
The kernel uses the TCAA_ACT_COUNT attribute to tell the user how many
actions are put in a single batch. As such user space app knows how long
to iterate (independent of the type of action being dumped)
instead of hardcoded maximum of 32.
Some results dumping 1.5M actions, first unpatched tc which the
kernel doesn't help:
prompt$ time -p tc actions ls action gact | grep index | wc -l
1500000
real 1388.43
user 2.07
sys 1386.79
Now lets see a patched tc which sets the correct flags when requesting
a dump:
prompt$ time -p updatedtc actions ls action gact | grep index | wc -l
1500000
real 178.13
user 2.02
sys 176.96
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
include/uapi/linux/rtnetlink.h | 21 +++++++++++++++++--
net/sched/act_api.c | 46 +++++++++++++++++++++++++++++++++---------
2 files changed, 55 insertions(+), 12 deletions(-)
diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
index cce0613..d7d28ec 100644
--- a/include/uapi/linux/rtnetlink.h
+++ b/include/uapi/linux/rtnetlink.h
@@ -674,10 +674,27 @@ struct tcamsg {
unsigned char tca__pad1;
unsigned short tca__pad2;
};
+
+enum {
+ TCAA_UNSPEC,
+ TCAA_ACT_TAB,
+#define TCA_ACT_TAB TCAA_ACT_TAB
+ TCAA_ACT_FLAGS,
+ TCAA_ACT_COUNT,
+ __TCAA_MAX,
+#define TCAA_MAX (__TCAA_MAX - 1)
+};
+
#define TA_RTA(r) ((struct rtattr*)(((char*)(r)) + NLMSG_ALIGN(sizeof(struct tcamsg))))
#define TA_PAYLOAD(n) NLMSG_PAYLOAD(n,sizeof(struct tcamsg))
-#define TCA_ACT_TAB 1 /* attr type must be >=1 */
-#define TCAA_MAX 1
+/* tcamsg flags stored in attribute TCAA_ACT_FLAGS
+ *
+ * ACT_LARGE_DUMP_ON user->kernel to request for larger than TCA_ACT_MAX_PRIO
+ * actions in a dump. All dump responses will contain the number of actions
+ * being dumped stored in for user app's consumption in TCAA_ACT_COUNT
+ *
+ */
+#define ACT_LARGE_DUMP_ON BIT(0)
/* New extended info filters for IFLA_EXT_MASK */
#define RTEXT_FILTER_VF (1 << 0)
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 82b1d48..f85b8fd 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -83,6 +83,7 @@ static int tcf_dump_walker(struct tcf_hashinfo *hinfo, struct sk_buff *skb,
struct netlink_callback *cb)
{
int err = 0, index = -1, i = 0, s_i = 0, n_i = 0;
+ unsigned short act_flags = cb->args[2];
struct nlattr *nest;
spin_lock_bh(&hinfo->lock);
@@ -111,14 +112,18 @@ static int tcf_dump_walker(struct tcf_hashinfo *hinfo, struct sk_buff *skb,
}
nla_nest_end(skb, nest);
n_i++;
- if (n_i >= TCA_ACT_MAX_PRIO)
+ if (!(act_flags & ACT_LARGE_DUMP_ON) &&
+ n_i >= TCA_ACT_MAX_PRIO)
goto done;
}
}
done:
spin_unlock_bh(&hinfo->lock);
- if (n_i)
+ if (n_i) {
cb->args[0] += n_i;
+ if (act_flags & ACT_LARGE_DUMP_ON)
+ cb->args[1] = n_i;
+ }
return n_i;
nla_put_failure:
@@ -993,11 +998,15 @@ static int tcf_action_add(struct net *net, struct nlattr *nla,
return tcf_add_notify(net, n, &actions, portid);
}
+static const struct nla_policy tcaa_policy[TCAA_MAX + 1] = {
+ [TCAA_ACT_FLAGS] = { .type = NLA_U32 },
+};
+
static int tc_ctl_action(struct sk_buff *skb, struct nlmsghdr *n,
struct netlink_ext_ack *extack)
{
struct net *net = sock_net(skb->sk);
- struct nlattr *tca[TCA_ACT_MAX + 1];
+ struct nlattr *tca[TCAA_MAX + 1];
u32 portid = skb ? NETLINK_CB(skb).portid : 0;
int ret = 0, ovr = 0;
@@ -1005,7 +1014,7 @@ static int tc_ctl_action(struct sk_buff *skb, struct nlmsghdr *n,
!netlink_capable(skb, CAP_NET_ADMIN))
return -EPERM;
- ret = nlmsg_parse(n, sizeof(struct tcamsg), tca, TCA_ACT_MAX, NULL,
+ ret = nlmsg_parse(n, sizeof(struct tcamsg), tca, TCAA_MAX, tcaa_policy,
extack);
if (ret < 0)
return ret;
@@ -1046,16 +1055,12 @@ static int tc_ctl_action(struct sk_buff *skb, struct nlmsghdr *n,
return ret;
}
-static struct nlattr *find_dump_kind(const struct nlmsghdr *n)
+static struct nlattr *find_dump_kind(struct nlattr **nla)
{
struct nlattr *tb1, *tb2[TCA_ACT_MAX + 1];
struct nlattr *tb[TCA_ACT_MAX_PRIO + 1];
- struct nlattr *nla[TCAA_MAX + 1];
struct nlattr *kind;
- if (nlmsg_parse(n, sizeof(struct tcamsg), nla, TCAA_MAX,
- NULL, NULL) < 0)
- return NULL;
tb1 = nla[TCA_ACT_TAB];
if (tb1 == NULL)
return NULL;
@@ -1081,9 +1086,18 @@ static int tc_dump_action(struct sk_buff *skb, struct netlink_callback *cb)
struct nlattr *nest;
struct tc_action_ops *a_o;
int ret = 0;
+ struct nlattr *tcaa[TCAA_MAX + 1];
struct tcamsg *t = (struct tcamsg *) nlmsg_data(cb->nlh);
- struct nlattr *kind = find_dump_kind(cb->nlh);
+ struct nlattr *kind = NULL;
+ struct nlattr *count_attr = NULL;
+ u32 act_flags = 0;
+
+ ret = nlmsg_parse(cb->nlh, sizeof(struct tcamsg), tcaa, TCAA_MAX,
+ tcaa_policy, NULL);
+ if (ret < 0)
+ return ret;
+ kind = find_dump_kind(tcaa);
if (kind == NULL) {
pr_info("tc_dump_action: action bad kind\n");
return 0;
@@ -1093,14 +1107,23 @@ static int tc_dump_action(struct sk_buff *skb, struct netlink_callback *cb)
if (a_o == NULL)
return 0;
+ if (tcaa[TCAA_ACT_FLAGS])
+ act_flags = nla_get_u32(tcaa[TCAA_ACT_FLAGS]);
+
nlh = nlmsg_put(skb, NETLINK_CB(cb->skb).portid, cb->nlh->nlmsg_seq,
cb->nlh->nlmsg_type, sizeof(*t), 0);
if (!nlh)
goto out_module_put;
+
+ cb->args[2] = act_flags;
+
t = nlmsg_data(nlh);
t->tca_family = AF_UNSPEC;
t->tca__pad1 = 0;
t->tca__pad2 = 0;
+ count_attr = nla_reserve(skb, TCAA_ACT_COUNT, sizeof(u32));
+ if (!count_attr)
+ goto out_module_put;
nest = nla_nest_start(skb, TCA_ACT_TAB);
if (nest == NULL)
@@ -1113,6 +1136,8 @@ static int tc_dump_action(struct sk_buff *skb, struct netlink_callback *cb)
if (ret > 0) {
nla_nest_end(skb, nest);
ret = skb->len;
+ memcpy(nla_data(count_attr), &cb->args[1], sizeof(u32));
+ cb->args[1] = 0;
} else
nlmsg_trim(skb, b);
--
1.9.1
^ permalink raw reply related
* [PATCH net-next v5 0/2] net sched actions: improve action dump performance
From: Jamal Hadi Salim @ 2017-04-20 13:06 UTC (permalink / raw)
To: davem; +Cc: jiri, xiyou.wangcong, eric.dumazet, netdev, Jamal Hadi Salim
From: Jamal Hadi Salim <jhs@mojatatu.com>
Changes since v4:
1) Eric D.
pointed out that when all skb space is used up by the dump
there will be no space to insert the TCAA_ACT_COUNT attribute.
2) Jiri:
i) Change:
enum {
TCAA_UNSPEC,
TCAA_ACT_TAB,
TCAA_ACT_FLAGS,
TCAA_ACT_COUNT,
TCAA_ACT_TIME_FILTER,
__TCAA_MAX
};
#define TCAA_MAX (__TCAA_MAX - 1)
#define ACT_LARGE_DUMP_ON (1 << 0)
to:
enum {
TCAA_UNSPEC,
TCAA_ACT_TAB,
#define TCA_ACT_TAB TCAA_ACT_TAB
TCAA_ACT_FLAGS,
TCAA_ACT_COUNT,
__TCAA_MAX,
#define TCAA_MAX (__TCAA_MAX - 1)
};
#define ACT_LARGE_DUMP_ON BIT(0)
Jiri plans to followup with the rest of the code to make the
style consistent.
ii) Rename attribute TCAA_ACT_TIME_FILTER --> TCAA_ACT_TIME_DELTA
iii) Rename variable jiffy_filter --> jiffy_since
iv) Rename msecs_filter --> msecs_since
v) get rid of unused cb->args[0] and rename cb->args[4] to cb->args[0]
Jamal Hadi Salim (2):
net sched actions: dump more than TCA_ACT_MAX_PRIO actions per batch
net sched actions: add time filter for action dumping
include/uapi/linux/rtnetlink.h | 22 ++++++++++++--
net/sched/act_api.c | 66 +++++++++++++++++++++++++++++++++++-------
2 files changed, 75 insertions(+), 13 deletions(-)
--
1.9.1
^ permalink raw reply
* Re: [PATCH net-next 0/3] l3mdev: Improve use with main table
From: Robert Shearman @ 2017-04-20 13:01 UTC (permalink / raw)
To: David Ahern, davem; +Cc: netdev
In-Reply-To: <e9cbd091-bb51-6f30-f31b-9080145caa93@cumulusnetworks.com>
On 13/04/17 15:36, David Ahern wrote:
> On 4/13/17 6:48 AM, Robert Shearman wrote:
>>> the patches look ok to me, but in testing them I see I refcnt problem.
>>> simple reproducer:
>>>
>>> ip li add red type vrf table 254
>>> ip link set dev eth1 vrf red
>>> ip addr add 127.0.0.1/8 dev red
>>> ip link set dev eth1 up
>>> ip li set red up
>>> ping -c1 -w1 -I red 127.0.0.1
>>> ip li del red
>>> --> hangs with 'uregister_netdevice: waiting for red to become free.'
>>
>> Right, I've reproduced the same and it occurs even without using the
>> main table and I believe it has been introduced within the last week.
>
> The cached dst on sockets is one known place that causes a hang on a
> delete. Basically the delete stalls until the sockets are closed. I have
> a patch for sk_rx_dst which is the one I chased down last week.
> sk_dst_cache is another possibility.
>
> Neither of those should be at play with the above example because the
> ping command runs and then exits.
Thanks for the pointers. My earlier assessment that this was something
recent turned out to be wrong. I've sent a patch against net that fixes
the issue.
Thanks,
Rob
^ permalink raw reply
* [PATCH net] ipv4: Avoid caching dsts when lookup skipped nh oif check
From: Robert Shearman @ 2017-04-20 12:58 UTC (permalink / raw)
To: davem; +Cc: netdev, David Ahern, Robert Shearman
David reported that doing the following:
ip li add red type vrf table 10
ip link set dev eth1 vrf red
ip addr add 127.0.0.1/8 dev red
ip link set dev eth1 up
ip li set red up
ping -c1 -w1 -I red 127.0.0.1
ip li del red
results in a hang with this message:
unregister_netdevice: waiting for red to become free. Usage count = 1
The problem is caused by caching the dst used for sending the packet
out of the specified interface on the route that the lookup returned
from the local table when the rule for the lookup in the local table
is ordered before the rule for lookups using l3mdevs. Thus the dst
could stay around until the route in the local table is deleted which
may be never.
Address the problem by not allocating a cacheable output dst if
FLOWI_FLAG_SKIP_NH_OIF is set and the nh device differs from the
device used for the dst.
Fixes: ebfc102c566d ("net: vrf: Flip IPv4 output path from FIB lookup hook to out hook")
Reported-by: David Ahern <dsa@cumulusnetworks.com>
Signed-off-by: Robert Shearman <rshearma@brocade.com>
---
net/ipv4/route.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index acd69cfe2951..f667783ffd19 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -2125,6 +2125,14 @@ static struct rtable *__mkroute_output(const struct fib_result *res,
fi = NULL;
}
+ /* If the flag to skip the nh oif check is set then the output
+ * device may not match the nh device, so cannot use or add to
+ * cache in that case.
+ */
+ if (unlikely(fl4->flowi4_flags & FLOWI_FLAG_SKIP_NH_OIF &&
+ FIB_RES_NH(*res).nh_dev != dev_out))
+ do_cache = false;
+
fnhe = NULL;
do_cache &= fi != NULL;
if (do_cache) {
--
2.1.4
^ permalink raw reply related
* macvlan: Fix device ref leak when purging bc_queue
From: Herbert Xu @ 2017-04-20 12:55 UTC (permalink / raw)
To: Joe.Ghalam; +Cc: davem, Clifford.Wichmann, netdev
In-Reply-To: <1492618528011.11322@Dell.com>
When a parent macvlan device is destroyed we end up purging its
broadcast queue without dropping the device reference count on
the packet source device. This causes the source device to linger.
This patch drops that reference count.
Fixes: 260916dfb48c ("macvlan: Fix potential use-after free for...")
Reported-by: Joe Ghalam <Joe.Ghalam@dell.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 9261722..b34eaaa 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -1139,6 +1139,7 @@ static int macvlan_port_create(struct net_device *dev)
static void macvlan_port_destroy(struct net_device *dev)
{
struct macvlan_port *port = macvlan_port_get_rtnl(dev);
+ struct sk_buff *skb;
dev->priv_flags &= ~IFF_MACVLAN_PORT;
netdev_rx_handler_unregister(dev);
@@ -1147,7 +1148,15 @@ static void macvlan_port_destroy(struct net_device *dev)
* but we need to cancel it and purge left skbs if any.
*/
cancel_work_sync(&port->bc_work);
- __skb_queue_purge(&port->bc_queue);
+
+ while ((skb = __skb_dequeue(&port->bc_queue))) {
+ const struct macvlan_dev *src = MACVLAN_SKB_CB(skb)->src;
+
+ if (src)
+ dev_put(src->dev);
+
+ kfree_skb(skb);
+ }
kfree(port);
}
--
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
* Re: [PATCH] brcm80211: brcmfmac: Ensure that incoming skb's are writable
From: Eric Dumazet @ 2017-04-20 12:49 UTC (permalink / raw)
To: James Hughes
Cc: netdev, Arend van Spriel, Franky Lin, Hante Meuleman, Kalle Valo
In-Reply-To: <20170420111651.10213-1-james.hughes@raspberrypi.org>
On Thu, 2017-04-20 at 12:16 +0100, James Hughes wrote:
> The driver was adding header information to incoming skb
> without ensuring the head was uncloned and hence writable.
>
> skb_cow_head has been used to ensure they are writable, however,
> this required some changes to error handling to ensure that
> if skb_cow_head failed it was not ignored.
>
> This really needs to be reviewed by someone who is more familiar
> with this code base to ensure any deallocation of skb's is
> still correct.
>
> Signed-off-by: James Hughes <james.hughes@raspberrypi.org>
> ---
You mention incoming skb in patch title, yet you change part of TX path.
Please split your changes in individual ones, to ease future bisections,
and ease backports as well.
Also, while it is known that TX path can deal with cloned skbs,
it is not clear why RX path has clones.
Most drivers allocate an skb, fill it, and pass it to network stack.
So please make sure all these changes are really needed.
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> index d138260..0e53c8a 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> @@ -2719,7 +2719,7 @@ static bool brcmf_sdio_prec_enq(struct pktq *q, struct sk_buff *pkt, int prec)
>
> static int brcmf_sdio_bus_txdata(struct device *dev, struct sk_buff *pkt)
> {
> - int ret = -EBADE;
> + int ret = -EBADE, err;
> uint prec;
> struct brcmf_bus *bus_if = dev_get_drvdata(dev);
> struct brcmf_sdio_dev *sdiodev = bus_if->bus_priv.sdio;
> @@ -2729,6 +2729,11 @@ static int brcmf_sdio_bus_txdata(struct device *dev, struct sk_buff *pkt)
> if (sdiodev->state != BRCMF_SDIOD_DATA)
> return -EIO;
>
> + err = skb_cow_head(pkt, bus->tx_hdrlen);
> +
> + if (err)
> + return err;
> +
> /* Add space for the header */
> skb_push(pkt, bus->tx_hdrlen);
> /* precondition: IS_ALIGNED((unsigned long)(pkt->data), 2) */
^ permalink raw reply
* Re: [PATCH v4 3/3] VSOCK: Add virtio vsock vsockmon hooks
From: Jorgen S. Hansen @ 2017-04-20 12:35 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: netdev@vger.kernel.org, Zhu Yanjun, Michael S. Tsirkin,
Gerard Garcia
In-Reply-To: <20170413161811.8953-4-stefanha@redhat.com>
>
> +/* Packet capture */
> +void virtio_transport_deliver_tap_pkt(struct virtio_vsock_pkt *pkt)
> +{
> + struct sk_buff *skb;
> + struct af_vsockmon_hdr *hdr;
> + unsigned char *t_hdr, *payload;
> +
> + skb = alloc_skb(sizeof(*hdr) + sizeof(pkt->hdr) + pkt->len,
> + GFP_ATOMIC);
So with the current API, an skb will always be allocated, even if there are no listeners? And we’ll copy the payload into the skb as well, if there is any. Would it make sense to introduce a check here (exposed as part of the vsock tap API), that skips all that in the case of no listeners? In the common case, there won’t be any listeners, so it would save some cycles.
> + if (!skb)
> + return; /* nevermind if we cannot capture the packet */
> +
> + hdr = (struct af_vsockmon_hdr *)skb_put(skb, sizeof(*hdr));
> +
> + /* pkt->hdr is little-endian so no need to byteswap here */
> + hdr->src_cid = pkt->hdr.src_cid;
> + hdr->src_port = pkt->hdr.src_port;
> + hdr->dst_cid = pkt->hdr.dst_cid;
> + hdr->dst_port = pkt->hdr.dst_port;
> +
> + hdr->transport = cpu_to_le16(AF_VSOCK_TRANSPORT_VIRTIO);
> + hdr->len = cpu_to_le16(sizeof(pkt->hdr));
> + hdr->reserved[0] = hdr->reserved[1] = 0;
> +
> + switch(cpu_to_le16(pkt->hdr.op)) {
> + case VIRTIO_VSOCK_OP_REQUEST:
> + case VIRTIO_VSOCK_OP_RESPONSE:
> + hdr->op = cpu_to_le16(AF_VSOCK_OP_CONNECT);
> + break;
> + case VIRTIO_VSOCK_OP_RST:
> + case VIRTIO_VSOCK_OP_SHUTDOWN:
> + hdr->op = cpu_to_le16(AF_VSOCK_OP_DISCONNECT);
> + break;
> + case VIRTIO_VSOCK_OP_RW:
> + hdr->op = cpu_to_le16(AF_VSOCK_OP_PAYLOAD);
> + break;
> + case VIRTIO_VSOCK_OP_CREDIT_UPDATE:
> + case VIRTIO_VSOCK_OP_CREDIT_REQUEST:
> + hdr->op = cpu_to_le16(AF_VSOCK_OP_CONTROL);
> + break;
> + default:
> + hdr->op = cpu_to_le16(AF_VSOCK_OP_UNKNOWN);
> + break;
> + }
> +
> + t_hdr = skb_put(skb, sizeof(pkt->hdr));
> + memcpy(t_hdr, &pkt->hdr, sizeof(pkt->hdr));
> +
> + if (pkt->len) {
> + payload = skb_put(skb, pkt->len);
> + memcpy(payload, pkt->buf, pkt->len);
> + }
> +
> + vsock_deliver_tap(skb);
> +}
> +EXPORT_SYMBOL_GPL(virtio_transport_deliver_tap_pkt);
> +
> static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
> struct virtio_vsock_pkt_info *info)
> {
> --
> 2.9.3
>
^ permalink raw reply
* Re: sctp: deny peeloff operation on asocs with threads sleeping on it
From: Salvatore Bonaccorso @ 2017-04-20 12:32 UTC (permalink / raw)
To: netdev
Cc: Alexander Popov, Ben Hutchings, Marcelo Ricardo Leitner, Xin Long,
David S. Miller
In-Reply-To: <20170420114405.mh7r5otmvpksacqd@lorien.valinor.li>
[-- Attachment #1: Type: text/plain, Size: 1050 bytes --]
Hi
According to the documentation I should have sent this to
netdev@vger.kernel.org rather than stable@vger.kernel.org.
Rationale: Whilst 00eff2ebbd229758e90659907724c14dd5a18339 went to
stable, the dfcb9f4f99f1e9a49e43398a7bfbf56927544af1 commit is
missing.
Full quoting, my original misleaded mail:
On Thu, Apr 20, 2017 at 01:44:05PM +0200, Salvatore Bonaccorso wrote:
> Hi
>
> Apparently the following commit
>
> dfcb9f4f99f1e9a49e43398a7bfbf56927544af1 (sctp: deny peeloff operation
> on asocs with threads sleeping on it)
>
> (was not CC'ed stable@vger.kernel.org, but was already applied in
> 3.2.87 and 3.16.42) is missing from 4.9:
>
> 2dcab598484185dea7ec22219c76dcdd59e3cb90 was included in 4.9.11 via
> 00eff2ebbd229758e90659907724c14dd5a18339 . But
> dfcb9f4f99f1e9a49e43398a7bfbf56927544af1 is missing from 4.9.
>
> This was assigned CVE-2017-6353.
>
> Reference:
> https://marc.info/?l=linux-netdev&m=148785309416337&w=2
> http://www.openwall.com/lists/oss-security/2017/02/27/2
>
> Regards,
> Salvatore
Regards,
Salvatore
[-- Attachment #2: 0001-sctp-deny-peeloff-operation-on-asocs-with-threads-sl.patch --]
[-- Type: text/x-diff, Size: 2474 bytes --]
>From dfcb9f4f99f1e9a49e43398a7bfbf56927544af1 Mon Sep 17 00:00:00 2001
From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Date: Thu, 23 Feb 2017 09:31:18 -0300
Subject: [PATCH] sctp: deny peeloff operation on asocs with threads sleeping
on it
commit 2dcab5984841 ("sctp: avoid BUG_ON on sctp_wait_for_sndbuf")
attempted to avoid a BUG_ON call when the association being used for a
sendmsg() is blocked waiting for more sndbuf and another thread did a
peeloff operation on such asoc, moving it to another socket.
As Ben Hutchings noticed, then in such case it would return without
locking back the socket and would cause two unlocks in a row.
Further analysis also revealed that it could allow a double free if the
application managed to peeloff the asoc that is created during the
sendmsg call, because then sctp_sendmsg() would try to free the asoc
that was created only for that call.
This patch takes another approach. It will deny the peeloff operation
if there is a thread sleeping on the asoc, so this situation doesn't
exist anymore. This avoids the issues described above and also honors
the syscalls that are already being handled (it can be multiple sendmsg
calls).
Joint work with Xin Long.
Fixes: 2dcab5984841 ("sctp: avoid BUG_ON on sctp_wait_for_sndbuf")
Cc: Alexander Popov <alex.popov@linux.com>
Cc: Ben Hutchings <ben@decadent.org.uk>
Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
net/sctp/socket.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index b532148..465a9c8 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -4862,6 +4862,12 @@ int sctp_do_peeloff(struct sock *sk, sctp_assoc_t id, struct socket **sockp)
if (!asoc)
return -EINVAL;
+ /* If there is a thread waiting on more sndbuf space for
+ * sending on this asoc, it cannot be peeled.
+ */
+ if (waitqueue_active(&asoc->wait))
+ return -EBUSY;
+
/* An association cannot be branched off from an already peeled-off
* socket, nor is this supported for tcp style sockets.
*/
@@ -7599,8 +7605,6 @@ static int sctp_wait_for_sndbuf(struct sctp_association *asoc, long *timeo_p,
*/
release_sock(sk);
current_timeo = schedule_timeout(current_timeo);
- if (sk != asoc->base.sk)
- goto do_error;
lock_sock(sk);
*timeo_p = current_timeo;
--
2.1.4
^ permalink raw reply related
* Re: [PATCH v4 1/3] VSOCK: Add vsockmon tap functions
From: Jorgen S. Hansen @ 2017-04-20 12:27 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: netdev@vger.kernel.org, Zhu Yanjun, Michael S. Tsirkin,
Gerard Garcia
In-Reply-To: <20170413161811.8953-2-stefanha@redhat.com>
> On Apr 13, 2017, at 6:18 PM, Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> +
> +static void __vsock_deliver_tap(struct sk_buff *skb)
> +{
> + int ret;
> + struct vsock_tap *tmp;
> +
> + list_for_each_entry_rcu(tmp, &vsock_tap_all, list) {
> + ret = __vsock_deliver_tap_skb(skb, tmp->dev);
> + if (unlikely(ret))
> + break;
> + }
> +
> + consume_skb(skb);
It looks like the caller will allocate the skb regardless of whether vsock_tap_all is empty, so shouldn’t the skb be consumed in vsock_deliver_tap?
> +}
> +
> +void vsock_deliver_tap(struct sk_buff *skb)
> +{
> + rcu_read_lock();
> +
> + if (unlikely(!list_empty(&vsock_tap_all)))
> + __vsock_deliver_tap(skb);
> +
> + rcu_read_unlock();
> +}
> +EXPORT_SYMBOL_GPL(vsock_deliver_tap);
> --
> 2.9.3
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox