* [PATCH net] Revert "net: Add driver helper functions to determine checksum offloadability"
From: Stephen Hemminger @ 2016-10-11 20:04 UTC (permalink / raw)
To: David Miller, Tom Herbert; +Cc: netdev
This reverts commit 6ae23ad36253a8033c5714c52b691b84456487c5.
The code has been in kernel since 4.4 but there are no in tree
code that uses. Unused code is broken code, remove it.
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
include/linux/netdevice.h | 78 --------------------------
net/core/dev.c | 136 ----------------------------------------------
2 files changed, 214 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 136ae6bb..793155e 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2649,71 +2649,6 @@ static inline void skb_gro_remcsum_cleanup(struct sk_buff *skb,
remcsum_unadjust((__sum16 *)ptr, grc->delta);
}
-struct skb_csum_offl_spec {
- __u16 ipv4_okay:1,
- ipv6_okay:1,
- encap_okay:1,
- ip_options_okay:1,
- ext_hdrs_okay:1,
- tcp_okay:1,
- udp_okay:1,
- sctp_okay:1,
- vlan_okay:1,
- no_encapped_ipv6:1,
- no_not_encapped:1;
-};
-
-bool __skb_csum_offload_chk(struct sk_buff *skb,
- const struct skb_csum_offl_spec *spec,
- bool *csum_encapped,
- bool csum_help);
-
-static inline bool skb_csum_offload_chk(struct sk_buff *skb,
- const struct skb_csum_offl_spec *spec,
- bool *csum_encapped,
- bool csum_help)
-{
- if (skb->ip_summed != CHECKSUM_PARTIAL)
- return false;
-
- return __skb_csum_offload_chk(skb, spec, csum_encapped, csum_help);
-}
-
-static inline bool skb_csum_offload_chk_help(struct sk_buff *skb,
- const struct skb_csum_offl_spec *spec)
-{
- bool csum_encapped;
-
- return skb_csum_offload_chk(skb, spec, &csum_encapped, true);
-}
-
-static inline bool skb_csum_off_chk_help_cmn(struct sk_buff *skb)
-{
- static const struct skb_csum_offl_spec csum_offl_spec = {
- .ipv4_okay = 1,
- .ip_options_okay = 1,
- .ipv6_okay = 1,
- .vlan_okay = 1,
- .tcp_okay = 1,
- .udp_okay = 1,
- };
-
- return skb_csum_offload_chk_help(skb, &csum_offl_spec);
-}
-
-static inline bool skb_csum_off_chk_help_cmn_v4_only(struct sk_buff *skb)
-{
- static const struct skb_csum_offl_spec csum_offl_spec = {
- .ipv4_okay = 1,
- .ip_options_okay = 1,
- .tcp_okay = 1,
- .udp_okay = 1,
- .vlan_okay = 1,
- };
-
- return skb_csum_offload_chk_help(skb, &csum_offl_spec);
-}
-
static inline int dev_hard_header(struct sk_buff *skb, struct net_device *dev,
unsigned short type,
const void *daddr, const void *saddr,
@@ -3957,19 +3892,6 @@ static inline bool can_checksum_protocol(netdev_features_t features,
}
}
-/* Map an ethertype into IP protocol if possible */
-static inline int eproto_to_ipproto(int eproto)
-{
- switch (eproto) {
- case htons(ETH_P_IP):
- return IPPROTO_IP;
- case htons(ETH_P_IPV6):
- return IPPROTO_IPV6;
- default:
- return -1;
- }
-}
-
#ifdef CONFIG_BUG
void netdev_rx_csum_fault(struct net_device *dev);
#else
diff --git a/net/core/dev.c b/net/core/dev.c
index f1fe26f..593f427 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -139,7 +139,6 @@
#include <linux/errqueue.h>
#include <linux/hrtimer.h>
#include <linux/netfilter_ingress.h>
-#include <linux/sctp.h>
#include <linux/crash_dump.h>
#include "net-sysfs.h"
@@ -2492,141 +2491,6 @@ out:
}
EXPORT_SYMBOL(skb_checksum_help);
-/* skb_csum_offload_check - Driver helper function to determine if a device
- * with limited checksum offload capabilities is able to offload the checksum
- * for a given packet.
- *
- * Arguments:
- * skb - sk_buff for the packet in question
- * spec - contains the description of what device can offload
- * csum_encapped - returns true if the checksum being offloaded is
- * encpasulated. That is it is checksum for the transport header
- * in the inner headers.
- * checksum_help - when set indicates that helper function should
- * call skb_checksum_help if offload checks fail
- *
- * Returns:
- * true: Packet has passed the checksum checks and should be offloadable to
- * the device (a driver may still need to check for additional
- * restrictions of its device)
- * false: Checksum is not offloadable. If checksum_help was set then
- * skb_checksum_help was called to resolve checksum for non-GSO
- * packets and when IP protocol is not SCTP
- */
-bool __skb_csum_offload_chk(struct sk_buff *skb,
- const struct skb_csum_offl_spec *spec,
- bool *csum_encapped,
- bool csum_help)
-{
- struct iphdr *iph;
- struct ipv6hdr *ipv6;
- void *nhdr;
- int protocol;
- u8 ip_proto;
-
- if (skb->protocol == htons(ETH_P_8021Q) ||
- skb->protocol == htons(ETH_P_8021AD)) {
- if (!spec->vlan_okay)
- goto need_help;
- }
-
- /* We check whether the checksum refers to a transport layer checksum in
- * the outermost header or an encapsulated transport layer checksum that
- * corresponds to the inner headers of the skb. If the checksum is for
- * something else in the packet we need help.
- */
- if (skb_checksum_start_offset(skb) == skb_transport_offset(skb)) {
- /* Non-encapsulated checksum */
- protocol = eproto_to_ipproto(vlan_get_protocol(skb));
- nhdr = skb_network_header(skb);
- *csum_encapped = false;
- if (spec->no_not_encapped)
- goto need_help;
- } else if (skb->encapsulation && spec->encap_okay &&
- skb_checksum_start_offset(skb) ==
- skb_inner_transport_offset(skb)) {
- /* Encapsulated checksum */
- *csum_encapped = true;
- switch (skb->inner_protocol_type) {
- case ENCAP_TYPE_ETHER:
- protocol = eproto_to_ipproto(skb->inner_protocol);
- break;
- case ENCAP_TYPE_IPPROTO:
- protocol = skb->inner_protocol;
- break;
- }
- nhdr = skb_inner_network_header(skb);
- } else {
- goto need_help;
- }
-
- switch (protocol) {
- case IPPROTO_IP:
- if (!spec->ipv4_okay)
- goto need_help;
- iph = nhdr;
- ip_proto = iph->protocol;
- if (iph->ihl != 5 && !spec->ip_options_okay)
- goto need_help;
- break;
- case IPPROTO_IPV6:
- if (!spec->ipv6_okay)
- goto need_help;
- if (spec->no_encapped_ipv6 && *csum_encapped)
- goto need_help;
- ipv6 = nhdr;
- nhdr += sizeof(*ipv6);
- ip_proto = ipv6->nexthdr;
- break;
- default:
- goto need_help;
- }
-
-ip_proto_again:
- switch (ip_proto) {
- case IPPROTO_TCP:
- if (!spec->tcp_okay ||
- skb->csum_offset != offsetof(struct tcphdr, check))
- goto need_help;
- break;
- case IPPROTO_UDP:
- if (!spec->udp_okay ||
- skb->csum_offset != offsetof(struct udphdr, check))
- goto need_help;
- break;
- case IPPROTO_SCTP:
- if (!spec->sctp_okay ||
- skb->csum_offset != offsetof(struct sctphdr, checksum))
- goto cant_help;
- break;
- case NEXTHDR_HOP:
- case NEXTHDR_ROUTING:
- case NEXTHDR_DEST: {
- u8 *opthdr = nhdr;
-
- if (protocol != IPPROTO_IPV6 || !spec->ext_hdrs_okay)
- goto need_help;
-
- ip_proto = opthdr[0];
- nhdr += (opthdr[1] + 1) << 3;
-
- goto ip_proto_again;
- }
- default:
- goto need_help;
- }
-
- /* Passed the tests for offloading checksum */
- return true;
-
-need_help:
- if (csum_help && !skb_shinfo(skb)->gso_size)
- skb_checksum_help(skb);
-cant_help:
- return false;
-}
-EXPORT_SYMBOL(__skb_csum_offload_chk);
-
__be16 skb_network_protocol(struct sk_buff *skb, int *depth)
{
__be16 type = skb->protocol;
--
2.9.3
^ permalink raw reply related
* Re: [PATCH net-next] tcp: Change txhash on some non-RTO retransmits
From: Yuchung Cheng @ 2016-10-11 19:49 UTC (permalink / raw)
To: Lawrence Brakmo; +Cc: netdev, Kernel Team, Eric Dumazet, Neal Cardwell
In-Reply-To: <20161011001841.1118150-1-brakmo@fb.com>
On Mon, Oct 10, 2016 at 5:18 PM, Lawrence Brakmo <brakmo@fb.com> wrote:
>
> The purpose of this patch is to help balance flows across paths. A new
> sysctl "tcp_retrans_txhash_prob" specifies the probability (0-100) that
> the txhash (IPv6 flowlabel) will be changed after a non-RTO retransmit.
> A probability is used in order to control how many flows are moved
> during a congestion event and prevent the congested path from becoming
> under utilized (which could occur if too many flows leave the current
> path). Txhash changes may be delayed in order to decrease the likelihood
> that it will trigger retransmists due to too much reordering.
>
> Another sysctl "tcp_retrans_txhash_mode" determines the behavior after
> RTOs. If the sysctl is 0, then after an RTO, only RTOs can trigger
> txhash changes. The idea is to decrease the likelihood of going back
> to a broken path. That is, we don't want flow balancing to trigger
> changes to broken paths. The drawback is that flow balancing does
> not work as well. If the sysctl is greater than 1, then we always
> do flow balancing, even after RTOs.
>
> Tested with packedrill tests (for correctness) and performance
> experiments with 2 and 3 paths. Performance experiments looked at
> aggregate goodput and fairness. For each run, we looked at the ratio of
> the goodputs for the fastest and slowest flows. These were averaged for
> all the runs. A fairness of 1 means all flows had the same goodput, a
> fairness of 2 means the fastest flow was twice as fast as the slowest
> flow.
>
> The setup for the performance experiments was 4 or 5 serves in a rack,
> 10G links. I tested various probabilities, but 20 seemed to have the
> best tradeoff for my setup (small RTTs).
>
> --- node1 -----
> sender --- switch --- node2 ----- switch ---- receiver
> --- node3 -----
>
> Scenario 1: One sender sends to one receiver through 2 routes (node1 or
> node 2). The output from node1 and node2 is 1G (1gbit/sec). With only 2
> flows, without flow balancing (prob=0) the average goodput is 1.6G vs.
> 1.9G with flow balancing due to 2 flows ending up in one link and either
> not moving and taking some time to move. Fairness was 1 in all cases.
> For 7 flows, goodput was 1.9G for all, but fairness was 1.5, 1.4 or 1.2
> for prob=0, prob=20,mode=0 and prob=20,mode=1 respectively. That is,
> flow balancing increased fairness.
>
> Scenario 2: One sender to one receiver, through 3 routes (node1,...
> node2). With 6 or 16 flows the goodput was the same for all, but
> fairness was 1.8, 1.5 and 1.2 respectively. Interestingly, the worst
> case fairness out of 10 runs were 2.2, 1.8 and 1.4 repectively. That is,
> prob=20,mode=1 improved average and worst case fairness.
I am wondering if we can build better API with routing layer to
implement this type of feature, instead of creeping the tx_rehashing
logic scatter in TCP. For example, we call dst_negative_advice on TCP
write timeouts.
On the patch itself, it seems aggressive to (attempt to) rehash every
post-RTO retranmission. Also you can just use ca_state (==CA_Loss) to
identify post-RTO retransmission directly.
is this an implementation of the Flow Bender ?
http://dl.acm.org/citation.cfm?id=2674985
>
> Scenario 3: One sender to one receiver, 2 routes, one route drops 50% of
> the packets. With 7 flows, goodput was the same 1.1G, but fairness was
> 1.8, 2.0 and 2.1 respectively. That is, if there is a bad route, then
> balancing, which does more re-routes, is less fair.
>
> Signed-off-by: Lawrence Brakmo <brakmo@fb.com>
> ---
> Documentation/networking/ip-sysctl.txt | 15 +++++++++++++++
> include/linux/tcp.h | 4 +++-
> include/net/tcp.h | 2 ++
> net/ipv4/sysctl_net_ipv4.c | 18 ++++++++++++++++++
> net/ipv4/tcp_input.c | 10 ++++++++++
> net/ipv4/tcp_output.c | 23 ++++++++++++++++++++++-
> net/ipv4/tcp_timer.c | 4 ++++
> 7 files changed, 74 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
> index 3db8c67..87a984c 100644
> --- a/Documentation/networking/ip-sysctl.txt
> +++ b/Documentation/networking/ip-sysctl.txt
> @@ -472,6 +472,21 @@ tcp_max_reordering - INTEGER
> if paths are using per packet load balancing (like bonding rr mode)
> Default: 300
>
> +tcp_retrans_txhash_mode - INTEGER
> + If zero, disable txhash recalculation due to non-RTO retransmissions
> + after an RTO. The idea is that broken paths will trigger an RTO and
> + we don't want going back to that path due to standard retransmissons
> + (flow balancing). The drawback is that balancing is less robust.
> + If greater than zero, can always (probabilistically) recalculate
> + txhash after non-RTO retransmissions.
> +
> +tcp_retrans_txhash_prob - INTEGER
> + Probability [0 to 100] that we will recalculate txhash when a
> + packet is resent not due to RTO (for RTO txhash is always recalculated).
> + The recalculation of the txhash may be delayed to decrease the
> + likelihood that reordering will trigger retransmissons.
> + The purpose is to help balance the flows among the possible paths.
> +
> tcp_retrans_collapse - BOOLEAN
> Bug-to-bug compatibility with some broken printers.
> On retransmit try to send bigger packets to work around bugs in
> diff --git a/include/linux/tcp.h b/include/linux/tcp.h
> index a17ae7b..e0e3b7d 100644
> --- a/include/linux/tcp.h
> +++ b/include/linux/tcp.h
> @@ -214,7 +214,9 @@ struct tcp_sock {
> } rack;
> u16 advmss; /* Advertised MSS */
> u8 rate_app_limited:1, /* rate_{delivered,interval_us} limited? */
> - unused:7;
> + txhash_rto:1, /* If set, don't do flow balancing */
> + txhash_want:1, /* We want to change txhash when safe */
> + unused:5;
> u8 nonagle : 4,/* Disable Nagle algorithm? */
> thin_lto : 1,/* Use linear timeouts for thin streams */
> thin_dupack : 1,/* Fast retransmit on first dupack */
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index f83b7f2..3abd304 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -271,6 +271,8 @@ extern int sysctl_tcp_autocorking;
> extern int sysctl_tcp_invalid_ratelimit;
> extern int sysctl_tcp_pacing_ss_ratio;
> extern int sysctl_tcp_pacing_ca_ratio;
> +extern int sysctl_tcp_retrans_txhash_prob;
> +extern int sysctl_tcp_retrans_txhash_mode;
>
> extern atomic_long_t tcp_memory_allocated;
> extern struct percpu_counter tcp_sockets_allocated;
> diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
> index 1cb67de..00d6f26 100644
> --- a/net/ipv4/sysctl_net_ipv4.c
> +++ b/net/ipv4/sysctl_net_ipv4.c
> @@ -28,6 +28,7 @@
> static int zero;
> static int one = 1;
> static int four = 4;
> +static int hundred = 100;
> static int thousand = 1000;
> static int gso_max_segs = GSO_MAX_SEGS;
> static int tcp_retr1_max = 255;
> @@ -624,6 +625,23 @@ static struct ctl_table ipv4_table[] = {
> .proc_handler = proc_dointvec_ms_jiffies,
> },
> {
> + .procname = "tcp_retrans_txhash_prob",
> + .data = &sysctl_tcp_retrans_txhash_prob,
> + .maxlen = sizeof(int),
> + .mode = 0644,
> + .proc_handler = proc_dointvec_minmax,
> + .extra1 = &zero,
> + .extra2 = &hundred,
> + },
> + {
> + .procname = "tcp_retrans_txhash_mode",
> + .data = &sysctl_tcp_retrans_txhash_mode,
> + .maxlen = sizeof(int),
> + .mode = 0644,
> + .proc_handler = proc_dointvec_minmax,
> + .extra1 = &zero,
> + },
> + {
> .procname = "icmp_msgs_per_sec",
> .data = &sysctl_icmp_msgs_per_sec,
> .maxlen = sizeof(int),
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index a27b9c0..fed5366 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -101,6 +101,9 @@ int sysctl_tcp_moderate_rcvbuf __read_mostly = 1;
> int sysctl_tcp_early_retrans __read_mostly = 3;
> int sysctl_tcp_invalid_ratelimit __read_mostly = HZ/2;
>
> +int sysctl_tcp_retrans_txhash_prob __read_mostly;
> +int sysctl_tcp_retrans_txhash_mode __read_mostly;
> +
> #define FLAG_DATA 0x01 /* Incoming frame contained data. */
> #define FLAG_WIN_UPDATE 0x02 /* Incoming ACK was a window update. */
> #define FLAG_DATA_ACKED 0x04 /* This ACK acknowledged new data. */
> @@ -3674,6 +3677,13 @@ static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag)
> flag |= tcp_clean_rtx_queue(sk, prior_fackets, prior_snd_una, &acked,
> &sack_state, &now);
>
> + /* Check if we should set txhash (would not cause reordering) */
> + if (tp->txhash_want &&
> + (tp->packets_out - tp->sacked_out) < tp->reordering) {
> + sk_set_txhash(sk);
> + tp->txhash_want = 0;
> + }
> +
> if (tcp_ack_is_dubious(sk, flag)) {
> is_dupack = !(flag & (FLAG_SND_UNA_ADVANCED | FLAG_NOT_DUP));
> tcp_fastretrans_alert(sk, acked, is_dupack, &flag, &rexmit);
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index 896e9df..58490ac 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -2738,9 +2738,30 @@ int tcp_retransmit_skb(struct sock *sk, struct sk_buff *skb, int segs)
> tp->retrans_out += tcp_skb_pcount(skb);
>
> /* Save stamp of the first retransmit. */
> - if (!tp->retrans_stamp)
> + if (!tp->retrans_stamp) {
> tp->retrans_stamp = tcp_skb_timestamp(skb);
>
> + /* Determine if we should reset hash, only done once
> + * per recovery
> + */
> + if ((!tp->txhash_rto ||
> + sysctl_tcp_retrans_txhash_mode > 0) &&
> + sk->sk_txhash &&
> + (prandom_u32_max(100) <
> + sysctl_tcp_retrans_txhash_prob)) {
> + /* If not too much reordering, or RTT is
> + * small enough that we don't care about
> + * reordering, then change it now.
> + * Else, wait until it is safe.
> + */
> + if ((tp->packets_out - tp->sacked_out) <
> + tp->reordering)
I don't parse this logic ... suppose reordering is 100 (not uncommon
today due to the last packet being delivered slightly earlier than the
rest), and cwnd==packets_out =~200,we only want to rehash until half
of the packets are sacked, so we are still rehashing even when
reordering is heavy?
also where do we check RTT is small?
> + sk_set_txhash(sk);
> + else
> + tp->txhash_want = 1;
> + }
> + }
> +
> } else if (err != -EBUSY) {
> NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPRETRANSFAIL);
> }
> diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
> index 3ea1cf8..e66baad 100644
> --- a/net/ipv4/tcp_timer.c
> +++ b/net/ipv4/tcp_timer.c
> @@ -186,6 +186,8 @@ static int tcp_write_timeout(struct sock *sk)
>
> if ((1 << sk->sk_state) & (TCPF_SYN_SENT | TCPF_SYN_RECV)) {
> if (icsk->icsk_retransmits) {
> + tp->txhash_rto = 1;
> + tp->txhash_want = 0;
> dst_negative_advice(sk);
> if (tp->syn_fastopen || tp->syn_data)
> tcp_fastopen_cache_set(sk, 0, NULL, true, 0);
> @@ -218,6 +220,8 @@ static int tcp_write_timeout(struct sock *sk)
> } else {
> sk_rethink_txhash(sk);
> }
> + tp->txhash_rto = 1;
> + tp->txhash_want = 0;
>
> retry_until = net->ipv4.sysctl_tcp_retries2;
> if (sock_flag(sk, SOCK_DEAD)) {
> --
> 2.9.3
>
^ permalink raw reply
* Re: [PATCH net-next] mlx5: Add MLX5_SET64_VCHK to fix BUILD_BUG_ON
From: Leon Romanovsky @ 2016-10-11 19:40 UTC (permalink / raw)
To: Saeed Mahameed
Cc: Tom Herbert, David Laight, davem@davemloft.net,
netdev@vger.kernel.org, saeedm@mellanox.com, kernel-team@fb.com
In-Reply-To: <CALx6S346RSTg6Knik5J4-BdvnTLrx28y-ADCt9ga344OFq4V0w@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1261 bytes --]
On Tue, Oct 11, 2016 at 08:46:45AM -0700, Tom Herbert wrote:
> On Tue, Oct 11, 2016 at 4:57 AM, Saeed Mahameed
> <saeedm@dev.mellanox.co.il> wrote:
> > On Tue, Oct 11, 2016 at 7:50 PM, David Laight <David.Laight@aculab.com> wrote:
> >> From: Tom Herbert
> >>> Sent: 11 October 2016 05:22
> >> ...
> >>> Fix is to create MLX5_SET64_VCHK that takes an additional argument
> >>> that is a constant. There are two callers of MLX5_SET64 that are
> >>> trying to get a variable offset, change those to call MLX5_SET64_VCHK
> >>> passing pas[0] as the argument to use in the offset check.
> >>
> >> I think I'd separate the array index instead.
> >> Something like:
> >>
> >> #define MLX5_SET64_INDEXED(typ, p, fld, ndx, v) do { \
> >> BUILD_BUG_ON(__mlx5_bit_off(typ, fld) % 64); \
> >> __MLX5_SET64(typ, p, fld[ndx], v); \
> >> } while (0)
> >>
> >> David
> >
> > Yes, I think this looks more natural, but instead MLX5_SET64_INDEXED,
> > I prefer to have 2 macros
> > MLX5_SET64(typ, p, fld, v) and MLX5_ARRAY_SET64(typ, p, fld, idx, v).
> >
> > Tom, do you want me to fix it ?
> >
> Please do.
Saeed,
Do you success to send this patch before -rc1 is released? So Linus's
-rc1 will be clean from such build error.
>
> > Thanks,
> > Saeed.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: RFH: problems with adjacency graph
From: David Ahern @ 2016-10-11 19:08 UTC (permalink / raw)
To: Jiri Pirko; +Cc: vfalico, Nikolay Aleksandrov, roopa, netdev@vger.kernel.org
In-Reply-To: <20161011065432.GA2018@nanopsycho>
On 10/11/16 12:54 AM, Jiri Pirko wrote:
>>
>> It seems like the complete mesh is not really needed, but cscope shows spectrum, ixgbe and bonding all using the for_each upper and lower device macros.
>>
>> Suggestions?
>
> Well other possibility is to traverse the tree recursively. But that is
> exactly why the colided lists of all uppers/lowers were introduced to
> avoid this.
The simpler approach is to remove all_adj_list completely and iteratively walk the lower and upper lists in adj_list for the macros that walk all lower and all upper devices. Maintaining a complete linear list per netdev of all lower devices and all upper devices is just not going to work given all combinations of enslave orderings and considering stacks as high as 6 deep. As far as I can tell the netdev_for_each_all_lower_dev, netdev_for_each_all_lower_dev_rcu, and netdev_for_each_all_upper_dev_rcu macros are all used in the slow path so it should be ok. Removing all_adj_list significantly simplifies the dev insert and remove code.
^ permalink raw reply
* Re: [PATCH] IB/ipoib: move back the IB LL address into the hard header
From: Doug Ledford @ 2016-10-11 18:50 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Paolo Abeni, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Sean Hefty,
Hal Rosenstock, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161011183019.GC20253-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 1038 bytes --]
On 10/11/2016 2:30 PM, Jason Gunthorpe wrote:
> On Tue, Oct 11, 2016 at 02:17:51PM -0400, Doug Ledford wrote:
>
>> Well, not exactly. Even if we put 65520 into the scripts, the kernel
>> will silently drop it down to 65504. It actually won't require anyone
>> change anything, they just won't get the full value. I experimented
>> with this in the past for other reasons and an overly large MTU setting
>> just resulted in the max MTU. I don't know if that's changed, but if it
>> still works that way, this is much less of an issue than it might
>> otherwise be.
>
> So it is just docs and relying on PMTU? That is not as bad..
>
> Still would be nice to avoid if at all possible..
I agree, but we have a test getting ready to commence. We'll know
shortly how much the reduced MTU effects things because they aren't
going to alter any of their setup, just put the new kernel in place, and
see what happens.
--
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
GPG Key ID: 0E572FDD
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]
^ permalink raw reply
* saddr based blackhole/unreachable route
From: Bjørnar Ness @ 2016-10-11 18:39 UTC (permalink / raw)
To: netdev
Hello, netdev
In a typical setup (eth0=internet, eth1=lan) i populate routing table
100 with saddrs I want
dropped, and: "ip ru a pref 100 lookup table 100"
What I would expect to see is packets with a saddr in table 100 coming
in eth0 will go out eth1,
with replies beeing dropped, but I do not see the packets going out eth1 at all.
Have tried searching and following the fib codepath, but have still
not managed to understand
what is really going on here.
Is the saddr looked up in the routing table?
Why dont I get icmp unreachable for unreachable routes?
Is tcpdump tricking me here?
I like the behavior, I just don't know if I can trust it.
Kernel 4.8.1
Regards,
--
Bj(/)rnar
^ permalink raw reply
* Re: [PATCH] IB/ipoib: move back the IB LL address into the hard header
From: Jason Gunthorpe @ 2016-10-11 18:30 UTC (permalink / raw)
To: Doug Ledford
Cc: Paolo Abeni, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Sean Hefty,
Hal Rosenstock, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <073e0007-a43b-134d-ba7e-b290304be585-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Tue, Oct 11, 2016 at 02:17:51PM -0400, Doug Ledford wrote:
> Well, not exactly. Even if we put 65520 into the scripts, the kernel
> will silently drop it down to 65504. It actually won't require anyone
> change anything, they just won't get the full value. I experimented
> with this in the past for other reasons and an overly large MTU setting
> just resulted in the max MTU. I don't know if that's changed, but if it
> still works that way, this is much less of an issue than it might
> otherwise be.
So it is just docs and relying on PMTU? That is not as bad..
Still would be nice to avoid if at all possible..
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] IB/ipoib: move back the IB LL address into the hard header
From: Jason Gunthorpe @ 2016-10-11 18:27 UTC (permalink / raw)
To: Paolo Abeni
Cc: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Sean Hefty,
Hal Rosenstock, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1476209407.448.9.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Tue, Oct 11, 2016 at 08:10:07PM +0200, Paolo Abeni wrote:
> The first s/g fragment (the head buffer) is not allocated with the page
> allocator, so perhaps there is some not too difficult/costly way out of
> this.
Keep in mind, there is nothing magic about the 16 SGL limit, other
than we know all hardware supports it. That can be bumped up and most
hardware will support a higher value.
We'd just have to figure out if any hardware breaks, Mellanox and Intel
should be able to respond to that question.
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] IB/ipoib: move back the IB LL address into the hard header
From: Doug Ledford @ 2016-10-11 18:17 UTC (permalink / raw)
To: Paolo Abeni, Jason Gunthorpe
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Sean Hefty, Hal Rosenstock,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1476209407.448.9.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 1622 bytes --]
On 10/11/2016 2:10 PM, Paolo Abeni wrote:
> On Tue, 2016-10-11 at 12:01 -0600, Jason Gunthorpe wrote:
>>> AFAICS the max mtu is already underlying h/w dependent, how does such
>>> differences are currently coped by ? (I'm sorry I lack some/a lot of IB
>>> back-ground)
>>
>> It isn't h/w dependent. In CM mode the MTU is 65520 because that is
>> what is hard coded into the ipoib driver. We tell everyone to use that
>> number. Eg see RH's docs on the subject:
>>
>> https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Linux/7/html/Networking_Guide/sec-Configuring_IPoIB.html
>>
>> AFAIK, today everyone just wires that number into their scripts, so we
>> have to mass change everything to the smaller number.
Well, not exactly. Even if we put 65520 into the scripts, the kernel
will silently drop it down to 65504. It actually won't require anyone
change anything, they just won't get the full value. I experimented
with this in the past for other reasons and an overly large MTU setting
just resulted in the max MTU. I don't know if that's changed, but if it
still works that way, this is much less of an issue than it might
otherwise be.
>> That sounds
>> really hard, IMHO if there is any way to avoid it we should, even if
>> it is a little costly.
>
> Thank you for the details!
>
> The first s/g fragment (the head buffer) is not allocated with the page
> allocator, so perhaps there is some not too difficult/costly way out of
> this.
>
>
>
>
>
>
--
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
GPG Key ID: 0E572FDD
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]
^ permalink raw reply
* Re: [PATCH] IB/ipoib: move back the IB LL address into the hard header
From: Paolo Abeni @ 2016-10-11 18:10 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Doug Ledford, linux-rdma, Sean Hefty, Hal Rosenstock, netdev
In-Reply-To: <20161011180119.GB17319@obsidianresearch.com>
On Tue, 2016-10-11 at 12:01 -0600, Jason Gunthorpe wrote:
> > AFAICS the max mtu is already underlying h/w dependent, how does such
> > differences are currently coped by ? (I'm sorry I lack some/a lot of IB
> > back-ground)
>
> It isn't h/w dependent. In CM mode the MTU is 65520 because that is
> what is hard coded into the ipoib driver. We tell everyone to use that
> number. Eg see RH's docs on the subject:
>
> https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Linux/7/html/Networking_Guide/sec-Configuring_IPoIB.html
>
> AFAIK, today everyone just wires that number into their scripts, so we
> have to mass change everything to the smaller number. That sounds
> really hard, IMHO if there is any way to avoid it we should, even if
> it is a little costly.
Thank you for the details!
The first s/g fragment (the head buffer) is not allocated with the page
allocator, so perhaps there is some not too difficult/costly way out of
this.
^ permalink raw reply
* Re: [PATCH net 3/3] openvswitch: add NETIF_F_HW_VLAN_STAG_TX to internal dev
From: Eric Garver @ 2016-10-11 17:41 UTC (permalink / raw)
To: Jiri Benc; +Cc: netdev, pravin shelar
In-Reply-To: <688ddda46c331c1b85450d6f28b1441af6cee1de.1476111550.git.jbenc@redhat.com>
On Mon, Oct 10, 2016 at 05:02:44PM +0200, Jiri Benc wrote:
> The internal device does support 802.1AD offloading since 018c1dda5ff1
> ("openvswitch: 802.1AD Flow handling, actions, vlan parsing, netlink
> attributes").
>
> Signed-off-by: Jiri Benc <jbenc@redhat.com>
Acked-by: Eric Garver <e@erig.me>
^ permalink raw reply
* Re: [PATCH net] Panic when tc_lookup_action_n finds a partially initialized action.
From: Cong Wang @ 2016-10-11 17:51 UTC (permalink / raw)
To: Krister Johansen; +Cc: Jamal Hadi Salim, Linux Kernel Network Developers
In-Reply-To: <20161011092805.GA7837@templeofstupid.com>
On Tue, Oct 11, 2016 at 2:28 AM, Krister Johansen
<kjlx@templeofstupid.com> wrote:
> On Wed, Oct 05, 2016 at 11:01:38AM -0700, Cong Wang wrote:
>> Does the attached patch make any sense now? Our pernet init doesn't
>> rely on act_base, so even we have some race, the worst case is after
>> we initialize the pernet netns for an action but its ops still not
>> visible, which seems fine (at least no crash).
>
> I tried to reproduce the panic with this latest patch, but I am unable
> to do so. The one difference I notice between this patch, and the one I
Nice, so the crash is fixed. I will send out my patch formally.
> sent to the list, is that with yours it takes much longer before we get
> any output from the simultaneous launch of these containers. Presumably
> that's the extra latency added by allowing many extra modprobe calls to
> get spawned by request_module().
This is a different problem. When we register a pernet ops, the ops->init()
will be called on each container to initialize its pernet data structures,
this is why request_module() blocks on waiting for register_pernet_subsys()
to finish. As we discussed earlier, this could be solved or workaround by
loading modules prior to creating containers. Or if this really needs to be
fixed, it should be in register_pernet_subsys(), since it is not specific to
tc actions, we have so many places loading modules at run-time in
networking subsystem.
Thanks for testing!
^ permalink raw reply
* Re: [PATCH] IB/ipoib: move back the IB LL address into the hard header
From: Jason Gunthorpe @ 2016-10-11 18:01 UTC (permalink / raw)
To: Doug Ledford; +Cc: Paolo Abeni, linux-rdma, Sean Hefty, Hal Rosenstock, netdev
In-Reply-To: <22f61258-9bcf-0adc-f23f-79a4f1d50c6a@redhat.com>
On Tue, Oct 11, 2016 at 01:41:56PM -0400, Doug Ledford wrote:
> declare the header. The problem then became that the sg setup is such
> that we are limited to 16 4k pages for the sg array, so that header had
> to come out of the 64k maximum mtu.
Oh, that clarifies things..
Hum, so various options become:
- Use >=17 SGL entries when creating the QP. Is this possible
on common adapters?
- Use the FRWR infrastructure when necessary. Is there any chance
the majority of skbs will have at least two physically
continuous pages to make this overhead rare? Perhaps as a fall
back if many adaptors can do >=17 SGLs
- Pad the hard header out to 4k and discard the first page
when building the sgl
- Memcopy the first ~8k into a contiguous 8k region on send
- Move the pseudo header to the end so it can cross the page
barrier without needing a sgl entry. (probably impossible?)
>From Paolo
> AFAICS the max mtu is already underlying h/w dependent, how does such
> differences are currently coped by ? (I'm sorry I lack some/a lot of IB
> back-ground)
It isn't h/w dependent. In CM mode the MTU is 65520 because that is
what is hard coded into the ipoib driver. We tell everyone to use that
number. Eg see RH's docs on the subject:
https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Linux/7/html/Networking_Guide/sec-Configuring_IPoIB.html
AFAIK, today everyone just wires that number into their scripts, so we
have to mass change everything to the smaller number. That sounds
really hard, IMHO if there is any way to avoid it we should, even if
it is a little costly.
Jason
^ permalink raw reply
* [Patch net] net_sched: reorder pernet ops and act ops registrations
From: Cong Wang @ 2016-10-11 17:56 UTC (permalink / raw)
To: netdev; +Cc: kjlx, Cong Wang, Jamal Hadi Salim
Krister reported a kernel NULL pointer dereference after
tcf_action_init_1() invokes a_o->init(), it is a race condition
where one thread calling tcf_register_action() to initialize
the netns data after putting act ops in the global list and
the other thread searching the list and then calling
a_o->init(net, ...).
Fix this by moving the pernet ops registration before making
the action ops visible. This is fine because: a) we don't
rely on act_base in pernet ops->init(), b) in the worst case we
have a fully initialized netns but ops is still not ready so
new actions still can't be created.
Reported-by: Krister Johansen <kjlx@templeofstupid.com>
Tested-by: Krister Johansen <kjlx@templeofstupid.com>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
net/sched/act_api.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index c910217..a512b18 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -341,22 +341,25 @@ int tcf_register_action(struct tc_action_ops *act,
if (!act->act || !act->dump || !act->init || !act->walk || !act->lookup)
return -EINVAL;
+ /* We have to register pernet ops before making the action ops visible,
+ * otherwise tcf_action_init_1() could get a partially initialized
+ * netns.
+ */
+ ret = register_pernet_subsys(ops);
+ if (ret)
+ return ret;
+
write_lock(&act_mod_lock);
list_for_each_entry(a, &act_base, head) {
if (act->type == a->type || (strcmp(act->kind, a->kind) == 0)) {
write_unlock(&act_mod_lock);
+ unregister_pernet_subsys(ops);
return -EEXIST;
}
}
list_add_tail(&act->head, &act_base);
write_unlock(&act_mod_lock);
- ret = register_pernet_subsys(ops);
- if (ret) {
- tcf_unregister_action(act, ops);
- return ret;
- }
-
return 0;
}
EXPORT_SYMBOL(tcf_register_action);
@@ -367,8 +370,6 @@ int tcf_unregister_action(struct tc_action_ops *act,
struct tc_action_ops *a;
int err = -ENOENT;
- unregister_pernet_subsys(ops);
-
write_lock(&act_mod_lock);
list_for_each_entry(a, &act_base, head) {
if (a == act) {
@@ -378,6 +379,8 @@ int tcf_unregister_action(struct tc_action_ops *act,
}
}
write_unlock(&act_mod_lock);
+ if (!err)
+ unregister_pernet_subsys(ops);
return err;
}
EXPORT_SYMBOL(tcf_unregister_action);
--
2.1.0
^ permalink raw reply related
* Re: [PATCH] IB/ipoib: move back the IB LL address into the hard header
From: Paolo Abeni @ 2016-10-11 17:52 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Doug Ledford, Sean Hefty,
Hal Rosenstock, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161011174224.GA17319-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
On Tue, 2016-10-11 at 11:42 -0600, Jason Gunthorpe wrote:
> On Tue, Oct 11, 2016 at 07:37:32PM +0200, Paolo Abeni wrote:
> > On Tue, 2016-10-11 at 11:32 -0600, Jason Gunthorpe wrote:
> > > On Tue, Oct 11, 2016 at 07:15:44PM +0200, Paolo Abeni wrote:
> > >
> > > > Also the connected mode maximum mtu is reduced by 16 bytes to
> > > > cope with the increased hard header len.
> > >
> > > Changing the MTU is going to cause annoying interop problems, can you
> > > avoid this?
> >
> > I don't like changing the maximum MTU value, too, but I was unable to
> > find an alternative solution. The PMTU detection should protect against
> > such issues.
>
> It is more that PMTU, we have instructed all users that is the MTU
> number needed to enable CM mode, so it appears in documentation,
> scripts, etc.
AFAICS the max mtu is already underlying h/w dependent, how does such
differences are currently coped by ? (I'm sorry I lack some/a lot of IB
back-ground)
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH net 2/3] openvswitch: fix vlan subtraction from packet length
From: Eric Garver @ 2016-10-11 17:41 UTC (permalink / raw)
To: Jiri Benc; +Cc: netdev, pravin shelar
In-Reply-To: <310ec523056b4f4747e800333498ef27ad1248aa.1476111550.git.jbenc@redhat.com>
On Mon, Oct 10, 2016 at 05:02:43PM +0200, Jiri Benc wrote:
> When the packet has its vlan tag in skb->vlan_tci, the length of the VLAN
> header is not counted in skb->len. It doesn't make sense to subtract it.
>
> Fixes: 018c1dda5ff1 ("openvswitch: 802.1AD Flow handling, actions, vlan parsing, netlink attributes")
> Signed-off-by: Jiri Benc <jbenc@redhat.com>
Acked-by: Eric Garver <e@erig.me>
^ permalink raw reply
* Re: [PATCH] IB/ipoib: move back the IB LL address into the hard header
From: Doug Ledford @ 2016-10-11 17:41 UTC (permalink / raw)
To: Jason Gunthorpe, Paolo Abeni
Cc: linux-rdma, Sean Hefty, Hal Rosenstock, netdev
In-Reply-To: <20161011173216.GA16892@obsidianresearch.com>
[-- Attachment #1.1: Type: text/plain, Size: 945 bytes --]
On 10/11/2016 1:32 PM, Jason Gunthorpe wrote:
> On Tue, Oct 11, 2016 at 07:15:44PM +0200, Paolo Abeni wrote:
>
>> Also the connected mode maximum mtu is reduced by 16 bytes to
>> cope with the increased hard header len.
>
> Changing the MTU is going to cause annoying interop problems, can you
> avoid this?
(Paolo did the work I'm describing here, I'm just giving the explanation
he gave me):
Not using this particular solution I don't think. We tried it without
increasing the declared hard header length and it broke when dealing
with skb_clone/GSO paths. In order to make the LL pseudo header get
copied along with the rest of the encap and data on clone, we had to
declare the header. The problem then became that the sg setup is such
that we are limited to 16 4k pages for the sg array, so that header had
to come out of the 64k maximum mtu.
--
Doug Ledford <dledford@redhat.com>
GPG Key ID: 0E572FDD
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]
^ permalink raw reply
* Re: [PATCH] IB/ipoib: move back the IB LL address into the hard header
From: Jason Gunthorpe @ 2016-10-11 17:42 UTC (permalink / raw)
To: Paolo Abeni
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Doug Ledford, Sean Hefty,
Hal Rosenstock, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1476207452.448.2.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Tue, Oct 11, 2016 at 07:37:32PM +0200, Paolo Abeni wrote:
> On Tue, 2016-10-11 at 11:32 -0600, Jason Gunthorpe wrote:
> > On Tue, Oct 11, 2016 at 07:15:44PM +0200, Paolo Abeni wrote:
> >
> > > Also the connected mode maximum mtu is reduced by 16 bytes to
> > > cope with the increased hard header len.
> >
> > Changing the MTU is going to cause annoying interop problems, can you
> > avoid this?
>
> I don't like changing the maximum MTU value, too, but I was unable to
> find an alternative solution. The PMTU detection should protect against
> such issues.
It is more that PMTU, we have instructed all users that is the MTU
number needed to enable CM mode, so it appears in documentation,
scripts, etc.
There is really no way to re-use some of the existing alignment
padding or exceed 64k?
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH net 1/3] openvswitch: vlan: remove wrong likely statement
From: Eric Garver @ 2016-10-11 17:38 UTC (permalink / raw)
To: Jiri Benc; +Cc: netdev, pravin shelar
In-Reply-To: <a4b63fa787569359d227beecbdbf9b023081cffc.1476111550.git.jbenc@redhat.com>
On Mon, Oct 10, 2016 at 05:02:42PM +0200, Jiri Benc wrote:
> This code is called whenever flow key is being extracted from the packet.
> The packet may be as likely vlan tagged as not.
>
> Fixes: 018c1dda5ff1 ("openvswitch: 802.1AD Flow handling, actions, vlan parsing, netlink attributes")
> Signed-off-by: Jiri Benc <jbenc@redhat.com>
Acked-by: Eric Garver <e@erig.me>
^ permalink raw reply
* Re: [PATCH] IB/ipoib: move back the IB LL address into the hard header
From: Paolo Abeni @ 2016-10-11 17:37 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: linux-rdma, Doug Ledford, Sean Hefty, Hal Rosenstock, netdev
In-Reply-To: <20161011173216.GA16892@obsidianresearch.com>
On Tue, 2016-10-11 at 11:32 -0600, Jason Gunthorpe wrote:
> On Tue, Oct 11, 2016 at 07:15:44PM +0200, Paolo Abeni wrote:
>
> > Also the connected mode maximum mtu is reduced by 16 bytes to
> > cope with the increased hard header len.
>
> Changing the MTU is going to cause annoying interop problems, can you
> avoid this?
I don't like changing the maximum MTU value, too, but I was unable to
find an alternative solution. The PMTU detection should protect against
such issues.
^ permalink raw reply
* Re: [PATCH net] Panic when tc_lookup_action_n finds a partially initialized action.
From: Cong Wang @ 2016-10-11 17:36 UTC (permalink / raw)
To: Krister Johansen; +Cc: Jamal Hadi Salim, Linux Kernel Network Developers
In-Reply-To: <20161009061331.GB2677@templeofstupid.com>
On Sat, Oct 8, 2016 at 11:13 PM, Krister Johansen
<kjlx@templeofstupid.com> wrote:
> Hi Cong,
> Thanks for the follow-up.
>
> On Thu, Oct 06, 2016 at 12:01:15PM -0700, Cong Wang wrote:
>> On Wed, Oct 5, 2016 at 11:11 PM, Krister Johansen
>> > pernet_operations pointer. The code in register_pernet_subsys() makes
>> > no attempt to check for duplicates. If we add a pointer that's already
>> > in the list, and subsequently call unregister, the results seem
>> > undefined. It looks like we'll remove the pernet_operations for the
>> > existing action, assuming we don't corrupt the list in the process.
>> >
>> > Is this actually safe? If so, what corner case is the act->type /
>> > act->kind protecting us from?
>>
>> ops->type and ops->kind should be unique too, user-space already
>> relies on this (tc action ls action xxx). The code exists probably just
>> for sanity check.
>
> With that in mind, would it make sense to change the check to a WARN/BUG
> or some kind of assertion? I mistakenly inferred that it was possible to
> legtimately end up in this scenario.
Yes, it makes sense to me.
>> > Part of the desire to inhibit extra modprobe calls is that if hundreds
>> > of these all start at once on boot, it's really unnecessary to have all
>> > of the rest of them wait while lots of extra modprobe calls are forked
>> > by the kernel.
>>
>> You can tell systemd to load these modules before starting these
>> containers to avoid blocking, no?
>
> That was exactly what I did to work around the panic until I was able to
> get a patch together. The preload of the modules is still occurring,
> but I was hoping to excise that workaround entirely.
Or you can compile these modules into kernel, but I am not sure about
the dependencies. :-D
Thanks.
^ permalink raw reply
* Re: [PATCH] IB/ipoib: move back the IB LL address into the hard header
From: Jason Gunthorpe @ 2016-10-11 17:32 UTC (permalink / raw)
To: Paolo Abeni
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Doug Ledford, Sean Hefty,
Hal Rosenstock, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1dbd83dfe7f435eecc5bc460e901b47758280f30.1476206016.git.pabeni-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Tue, Oct 11, 2016 at 07:15:44PM +0200, Paolo Abeni wrote:
> Also the connected mode maximum mtu is reduced by 16 bytes to
> cope with the increased hard header len.
Changing the MTU is going to cause annoying interop problems, can you
avoid this?
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH] IB/ipoib: move back the IB LL address into the hard header
From: Paolo Abeni @ 2016-10-11 17:15 UTC (permalink / raw)
To: linux-rdma-u79uwXL29TY76Z2rM5mHXA
Cc: Doug Ledford, Sean Hefty, Hal Rosenstock,
netdev-u79uwXL29TY76Z2rM5mHXA
After the commit 9207f9d45b0a ("net: preserve IP control block
during GSO segmentation"), the GSO CB and the IPoIB CB conflict.
That destroy the IPoIB address information cached there,
causing a severe performance regression, as better described here:
http://marc.info/?l=linux-kernel&m=146787279825501&w=2
This change moves the data cached by the IPoIB driver from the
skb control lock into the IPoIB hard header, as done before
the commit 936d7de3d736 ("IPoIB: Stop lying about hard_header_len
and use skb->cb to stash LL addresses").
In order to avoid GRO issue, on packet reception, the IPoIB driver
stash into the skb a dummy pseudo header, so that the received
packets have actually a hard header matching the declared length.
Also the connected mode maximum mtu is reduced by 16 bytes to
cope with the increased hard header len.
After this commit, IPoIB performances are back to pre-regression
value.
Fixes: 9207f9d45b0a ("net: preserve IP control block during GSO segmentation")
Signed-off-by: Paolo Abeni <pabeni-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
drivers/infiniband/ulp/ipoib/ipoib.h | 24 ++++++++----
drivers/infiniband/ulp/ipoib/ipoib_cm.c | 17 ++++----
drivers/infiniband/ulp/ipoib/ipoib_ib.c | 12 +++---
drivers/infiniband/ulp/ipoib/ipoib_main.c | 54 ++++++++++++++++----------
drivers/infiniband/ulp/ipoib/ipoib_multicast.c | 6 ++-
5 files changed, 67 insertions(+), 46 deletions(-)
diff --git a/drivers/infiniband/ulp/ipoib/ipoib.h b/drivers/infiniband/ulp/ipoib/ipoib.h
index 9dbfcc0..5dd01fa 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib.h
+++ b/drivers/infiniband/ulp/ipoib/ipoib.h
@@ -63,12 +63,14 @@ enum ipoib_flush_level {
enum {
IPOIB_ENCAP_LEN = 4,
+ IPOIB_PSEUDO_LEN = 20,
+ IPOIB_HARD_LEN = IPOIB_ENCAP_LEN + IPOIB_PSEUDO_LEN,
IPOIB_UD_HEAD_SIZE = IB_GRH_BYTES + IPOIB_ENCAP_LEN,
IPOIB_UD_RX_SG = 2, /* max buffer needed for 4K mtu */
- IPOIB_CM_MTU = 0x10000 - 0x10, /* padding to align header to 16 */
- IPOIB_CM_BUF_SIZE = IPOIB_CM_MTU + IPOIB_ENCAP_LEN,
+ IPOIB_CM_MTU = 0x10000 - 0x20, /* padding to align header to 16 */
+ IPOIB_CM_BUF_SIZE = IPOIB_CM_MTU + IPOIB_HARD_LEN,
IPOIB_CM_HEAD_SIZE = IPOIB_CM_BUF_SIZE % PAGE_SIZE,
IPOIB_CM_RX_SG = ALIGN(IPOIB_CM_BUF_SIZE, PAGE_SIZE) / PAGE_SIZE,
IPOIB_RX_RING_SIZE = 256,
@@ -134,15 +136,21 @@ struct ipoib_header {
u16 reserved;
};
-struct ipoib_cb {
- struct qdisc_skb_cb qdisc_cb;
- u8 hwaddr[INFINIBAND_ALEN];
+struct ipoib_pseudo_header {
+ u8 hwaddr[INFINIBAND_ALEN];
};
-static inline struct ipoib_cb *ipoib_skb_cb(const struct sk_buff *skb)
+static inline void skb_add_pseudo_hdr(struct sk_buff *skb)
{
- BUILD_BUG_ON(sizeof(skb->cb) < sizeof(struct ipoib_cb));
- return (struct ipoib_cb *)skb->cb;
+ char *data = skb_push(skb, IPOIB_PSEUDO_LEN);
+
+ /*
+ * only the ipoib header is present now, make room for a dummy
+ * pseudo header and set skb field accordingly
+ */
+ memset(data, 0, IPOIB_PSEUDO_LEN);
+ skb_reset_mac_header(skb);
+ skb_pull(skb, IPOIB_HARD_LEN);
}
/* Used for all multicast joins (broadcast, IPv4 mcast and IPv6 mcast) */
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_cm.c b/drivers/infiniband/ulp/ipoib/ipoib_cm.c
index 4ad297d..1b04c8a 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_cm.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_cm.c
@@ -63,6 +63,8 @@ MODULE_PARM_DESC(cm_data_debug_level,
#define IPOIB_CM_RX_DELAY (3 * 256 * HZ)
#define IPOIB_CM_RX_UPDATE_MASK (0x3)
+#define IPOIB_CM_RX_RESERVE (ALIGN(IPOIB_HARD_LEN, 16) - IPOIB_ENCAP_LEN)
+
static struct ib_qp_attr ipoib_cm_err_attr = {
.qp_state = IB_QPS_ERR
};
@@ -146,15 +148,15 @@ static struct sk_buff *ipoib_cm_alloc_rx_skb(struct net_device *dev,
struct sk_buff *skb;
int i;
- skb = dev_alloc_skb(IPOIB_CM_HEAD_SIZE + 12);
+ skb = dev_alloc_skb(ALIGN(IPOIB_CM_HEAD_SIZE, 16));
if (unlikely(!skb))
return NULL;
/*
- * IPoIB adds a 4 byte header. So we need 12 more bytes to align the
+ * IPoIB adds a IPOIB_ENCAP_LEN byte header, this will align the
* IP header to a multiple of 16.
*/
- skb_reserve(skb, 12);
+ skb_reserve(skb, IPOIB_CM_RX_RESERVE);
mapping[0] = ib_dma_map_single(priv->ca, skb->data, IPOIB_CM_HEAD_SIZE,
DMA_FROM_DEVICE);
@@ -624,9 +626,9 @@ void ipoib_cm_handle_rx_wc(struct net_device *dev, struct ib_wc *wc)
if (wc->byte_len < IPOIB_CM_COPYBREAK) {
int dlen = wc->byte_len;
- small_skb = dev_alloc_skb(dlen + 12);
+ small_skb = dev_alloc_skb(dlen + IPOIB_CM_RX_RESERVE);
if (small_skb) {
- skb_reserve(small_skb, 12);
+ skb_reserve(small_skb, IPOIB_CM_RX_RESERVE);
ib_dma_sync_single_for_cpu(priv->ca, rx_ring[wr_id].mapping[0],
dlen, DMA_FROM_DEVICE);
skb_copy_from_linear_data(skb, small_skb->data, dlen);
@@ -663,8 +665,7 @@ void ipoib_cm_handle_rx_wc(struct net_device *dev, struct ib_wc *wc)
copied:
skb->protocol = ((struct ipoib_header *) skb->data)->proto;
- skb_reset_mac_header(skb);
- skb_pull(skb, IPOIB_ENCAP_LEN);
+ skb_add_pseudo_hdr(skb);
++dev->stats.rx_packets;
dev->stats.rx_bytes += skb->len;
@@ -1583,7 +1584,7 @@ int ipoib_cm_dev_init(struct net_device *dev)
max_srq_sge = min_t(int, IPOIB_CM_RX_SG, priv->ca->attrs.max_srq_sge);
ipoib_cm_create_srq(dev, max_srq_sge);
if (ipoib_cm_has_srq(dev)) {
- priv->cm.max_cm_mtu = max_srq_sge * PAGE_SIZE - 0x10;
+ priv->cm.max_cm_mtu = max_srq_sge * PAGE_SIZE - 0x20;
priv->cm.num_frags = max_srq_sge;
ipoib_dbg(priv, "max_cm_mtu = 0x%x, num_frags=%d\n",
priv->cm.max_cm_mtu, priv->cm.num_frags);
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_ib.c b/drivers/infiniband/ulp/ipoib/ipoib_ib.c
index be11d5d..830fecb 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_ib.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_ib.c
@@ -128,16 +128,15 @@ static struct sk_buff *ipoib_alloc_rx_skb(struct net_device *dev, int id)
buf_size = IPOIB_UD_BUF_SIZE(priv->max_ib_mtu);
- skb = dev_alloc_skb(buf_size + IPOIB_ENCAP_LEN);
+ skb = dev_alloc_skb(buf_size + IPOIB_HARD_LEN);
if (unlikely(!skb))
return NULL;
/*
- * IB will leave a 40 byte gap for a GRH and IPoIB adds a 4 byte
- * header. So we need 4 more bytes to get to 48 and align the
- * IP header to a multiple of 16.
+ * the IP header will be at IPOIP_HARD_LEN + IB_GRH_BYTES, that is
+ * 64 bytes aligned
*/
- skb_reserve(skb, 4);
+ skb_reserve(skb, sizeof(struct ipoib_pseudo_header));
mapping = priv->rx_ring[id].mapping;
mapping[0] = ib_dma_map_single(priv->ca, skb->data, buf_size,
@@ -253,8 +252,7 @@ static void ipoib_ib_handle_rx_wc(struct net_device *dev, struct ib_wc *wc)
skb_pull(skb, IB_GRH_BYTES);
skb->protocol = ((struct ipoib_header *) skb->data)->proto;
- skb_reset_mac_header(skb);
- skb_pull(skb, IPOIB_ENCAP_LEN);
+ skb_add_pseudo_hdr(skb);
++dev->stats.rx_packets;
dev->stats.rx_bytes += skb->len;
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c
index cc1c1b0..823a528 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
@@ -925,9 +925,12 @@ static void neigh_add_path(struct sk_buff *skb, u8 *daddr,
ipoib_neigh_free(neigh);
goto err_drop;
}
- if (skb_queue_len(&neigh->queue) < IPOIB_MAX_PATH_REC_QUEUE)
+ if (skb_queue_len(&neigh->queue) <
+ IPOIB_MAX_PATH_REC_QUEUE) {
+ /* put pseudoheader back on for next time */
+ skb_push(skb, IPOIB_PSEUDO_LEN);
__skb_queue_tail(&neigh->queue, skb);
- else {
+ } else {
ipoib_warn(priv, "queue length limit %d. Packet drop.\n",
skb_queue_len(&neigh->queue));
goto err_drop;
@@ -964,7 +967,7 @@ err_drop:
}
static void unicast_arp_send(struct sk_buff *skb, struct net_device *dev,
- struct ipoib_cb *cb)
+ struct ipoib_pseudo_header *phdr)
{
struct ipoib_dev_priv *priv = netdev_priv(dev);
struct ipoib_path *path;
@@ -972,16 +975,18 @@ static void unicast_arp_send(struct sk_buff *skb, struct net_device *dev,
spin_lock_irqsave(&priv->lock, flags);
- path = __path_find(dev, cb->hwaddr + 4);
+ path = __path_find(dev, phdr->hwaddr + 4);
if (!path || !path->valid) {
int new_path = 0;
if (!path) {
- path = path_rec_create(dev, cb->hwaddr + 4);
+ path = path_rec_create(dev, phdr->hwaddr + 4);
new_path = 1;
}
if (path) {
if (skb_queue_len(&path->queue) < IPOIB_MAX_PATH_REC_QUEUE) {
+ /* put pseudoheader back on for next time */
+ skb_push(skb, IPOIB_PSEUDO_LEN);
__skb_queue_tail(&path->queue, skb);
} else {
++dev->stats.tx_dropped;
@@ -1009,10 +1014,12 @@ static void unicast_arp_send(struct sk_buff *skb, struct net_device *dev,
be16_to_cpu(path->pathrec.dlid));
spin_unlock_irqrestore(&priv->lock, flags);
- ipoib_send(dev, skb, path->ah, IPOIB_QPN(cb->hwaddr));
+ ipoib_send(dev, skb, path->ah, IPOIB_QPN(phdr->hwaddr));
return;
} else if ((path->query || !path_rec_start(dev, path)) &&
skb_queue_len(&path->queue) < IPOIB_MAX_PATH_REC_QUEUE) {
+ /* put pseudoheader back on for next time */
+ skb_push(skb, IPOIB_PSEUDO_LEN);
__skb_queue_tail(&path->queue, skb);
} else {
++dev->stats.tx_dropped;
@@ -1026,13 +1033,15 @@ static int ipoib_start_xmit(struct sk_buff *skb, struct net_device *dev)
{
struct ipoib_dev_priv *priv = netdev_priv(dev);
struct ipoib_neigh *neigh;
- struct ipoib_cb *cb = ipoib_skb_cb(skb);
+ struct ipoib_pseudo_header *phdr;
struct ipoib_header *header;
unsigned long flags;
+ phdr = (struct ipoib_pseudo_header *) skb->data;
+ skb_pull(skb, sizeof(*phdr));
header = (struct ipoib_header *) skb->data;
- if (unlikely(cb->hwaddr[4] == 0xff)) {
+ if (unlikely(phdr->hwaddr[4] == 0xff)) {
/* multicast, arrange "if" according to probability */
if ((header->proto != htons(ETH_P_IP)) &&
(header->proto != htons(ETH_P_IPV6)) &&
@@ -1045,13 +1054,13 @@ static int ipoib_start_xmit(struct sk_buff *skb, struct net_device *dev)
return NETDEV_TX_OK;
}
/* Add in the P_Key for multicast*/
- cb->hwaddr[8] = (priv->pkey >> 8) & 0xff;
- cb->hwaddr[9] = priv->pkey & 0xff;
+ phdr->hwaddr[8] = (priv->pkey >> 8) & 0xff;
+ phdr->hwaddr[9] = priv->pkey & 0xff;
- neigh = ipoib_neigh_get(dev, cb->hwaddr);
+ neigh = ipoib_neigh_get(dev, phdr->hwaddr);
if (likely(neigh))
goto send_using_neigh;
- ipoib_mcast_send(dev, cb->hwaddr, skb);
+ ipoib_mcast_send(dev, phdr->hwaddr, skb);
return NETDEV_TX_OK;
}
@@ -1060,16 +1069,16 @@ static int ipoib_start_xmit(struct sk_buff *skb, struct net_device *dev)
case htons(ETH_P_IP):
case htons(ETH_P_IPV6):
case htons(ETH_P_TIPC):
- neigh = ipoib_neigh_get(dev, cb->hwaddr);
+ neigh = ipoib_neigh_get(dev, phdr->hwaddr);
if (unlikely(!neigh)) {
- neigh_add_path(skb, cb->hwaddr, dev);
+ neigh_add_path(skb, phdr->hwaddr, dev);
return NETDEV_TX_OK;
}
break;
case htons(ETH_P_ARP):
case htons(ETH_P_RARP):
/* for unicast ARP and RARP should always perform path find */
- unicast_arp_send(skb, dev, cb);
+ unicast_arp_send(skb, dev, phdr);
return NETDEV_TX_OK;
default:
/* ethertype not supported by IPoIB */
@@ -1086,11 +1095,13 @@ send_using_neigh:
goto unref;
}
} else if (neigh->ah) {
- ipoib_send(dev, skb, neigh->ah, IPOIB_QPN(cb->hwaddr));
+ ipoib_send(dev, skb, neigh->ah, IPOIB_QPN(phdr->hwaddr));
goto unref;
}
if (skb_queue_len(&neigh->queue) < IPOIB_MAX_PATH_REC_QUEUE) {
+ /* put pseudoheader back on for next time */
+ skb_push(skb, sizeof(*phdr));
spin_lock_irqsave(&priv->lock, flags);
__skb_queue_tail(&neigh->queue, skb);
spin_unlock_irqrestore(&priv->lock, flags);
@@ -1122,8 +1133,8 @@ static int ipoib_hard_header(struct sk_buff *skb,
unsigned short type,
const void *daddr, const void *saddr, unsigned len)
{
+ struct ipoib_pseudo_header *phdr;
struct ipoib_header *header;
- struct ipoib_cb *cb = ipoib_skb_cb(skb);
header = (struct ipoib_header *) skb_push(skb, sizeof *header);
@@ -1132,12 +1143,13 @@ static int ipoib_hard_header(struct sk_buff *skb,
/*
* we don't rely on dst_entry structure, always stuff the
- * destination address into skb->cb so we can figure out where
+ * destination address into skb hard header so we can figure out where
* to send the packet later.
*/
- memcpy(cb->hwaddr, daddr, INFINIBAND_ALEN);
+ phdr = (struct ipoib_pseudo_header *) skb_push(skb, sizeof(*phdr));
+ memcpy(phdr->hwaddr, daddr, INFINIBAND_ALEN);
- return sizeof *header;
+ return IPOIB_HARD_LEN;
}
static void ipoib_set_mcast_list(struct net_device *dev)
@@ -1759,7 +1771,7 @@ void ipoib_setup(struct net_device *dev)
dev->flags |= IFF_BROADCAST | IFF_MULTICAST;
- dev->hard_header_len = IPOIB_ENCAP_LEN;
+ dev->hard_header_len = IPOIB_HARD_LEN;
dev->addr_len = INFINIBAND_ALEN;
dev->type = ARPHRD_INFINIBAND;
dev->tx_queue_len = ipoib_sendq_size * 2;
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
index d3394b6..1909dd2 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
@@ -796,9 +796,11 @@ void ipoib_mcast_send(struct net_device *dev, u8 *daddr, struct sk_buff *skb)
__ipoib_mcast_add(dev, mcast);
list_add_tail(&mcast->list, &priv->multicast_list);
}
- if (skb_queue_len(&mcast->pkt_queue) < IPOIB_MAX_MCAST_QUEUE)
+ if (skb_queue_len(&mcast->pkt_queue) < IPOIB_MAX_MCAST_QUEUE) {
+ /* put pseudoheader back on for next time */
+ skb_push(skb, sizeof(struct ipoib_pseudo_header));
skb_queue_tail(&mcast->pkt_queue, skb);
- else {
+ } else {
++dev->stats.tx_dropped;
dev_kfree_skb_any(skb);
}
--
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* ATENCIÓN;
From: Sistemas administrador @ 2016-10-11 16:25 UTC (permalink / raw)
To: Recipients
ATENCIÓN;
Su buzón ha superado el límite de almacenamiento, que es de 5 GB definidos por el administrador, quien actualmente está ejecutando en 10.9GB, no puede ser capaz de enviar o recibir correo nuevo hasta que
vuelva a validar su buzón de correo electrónico. Para revalidar su buzón de correo, envíe la siguiente información a continuación:
nombre:
Nombre de usuario:
contraseña:
Confirmar contraseña:
E-mail:
teléfono:
Si usted no puede revalidar su buzón, el buzón se deshabilitará!
Disculpa las molestias.
Código de verificación: es: 006524
Correo Soporte Técnico © 2016
¡gracias
Sistemas administrador
^ permalink raw reply
* Re: [PATCH net-next] mlx5: Add MLX5_SET64_VCHK to fix BUILD_BUG_ON
From: Tom Herbert @ 2016-10-11 15:46 UTC (permalink / raw)
To: Saeed Mahameed
Cc: David Laight, davem@davemloft.net, netdev@vger.kernel.org,
saeedm@mellanox.com, kernel-team@fb.com
In-Reply-To: <CALzJLG_9Yr=jCPix2Oisxf22CLHxtYYyXejYxkBi-A85kZ_Shw@mail.gmail.com>
On Tue, Oct 11, 2016 at 4:57 AM, Saeed Mahameed
<saeedm@dev.mellanox.co.il> wrote:
> On Tue, Oct 11, 2016 at 7:50 PM, David Laight <David.Laight@aculab.com> wrote:
>> From: Tom Herbert
>>> Sent: 11 October 2016 05:22
>> ...
>>> Fix is to create MLX5_SET64_VCHK that takes an additional argument
>>> that is a constant. There are two callers of MLX5_SET64 that are
>>> trying to get a variable offset, change those to call MLX5_SET64_VCHK
>>> passing pas[0] as the argument to use in the offset check.
>>
>> I think I'd separate the array index instead.
>> Something like:
>>
>> #define MLX5_SET64_INDEXED(typ, p, fld, ndx, v) do { \
>> BUILD_BUG_ON(__mlx5_bit_off(typ, fld) % 64); \
>> __MLX5_SET64(typ, p, fld[ndx], v); \
>> } while (0)
>>
>> David
>
> Yes, I think this looks more natural, but instead MLX5_SET64_INDEXED,
> I prefer to have 2 macros
> MLX5_SET64(typ, p, fld, v) and MLX5_ARRAY_SET64(typ, p, fld, idx, v).
>
> Tom, do you want me to fix it ?
>
Please do.
> Thanks,
> Saeed.
^ 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