gmane.network.tipc.general

Re: [RFC PATCH net-next 0/5] introduce linkcongestion control algorithm

Jon Maloy — 2013-05-23T16:46:05

Good to see we agree on this. I am looking forwards to see your results.

///jon



------------------------------------------------------------------------------
Try New Relic Now & We'll Send You this Cool Shirt
New Relic is the only SaaS-based application performance monitoring service 
that delivers powerful full stack analytics. Optimize and monitor your
browser, app, & servers with just a few lines of code. Try New Relic
and get this awesome Nerd Life shirt! http://p.sf.net/sfu/newrelic_d2d_may

RDM and multicast flow control v2

Jon Maloy — 2013-05-23T16:41:01

Improvement to mechanism A. No need to send warnings to senders who shouldn't be affected.

Discussion items: 
1) Is A needed at all? I think so,because it would dramatically reduce the risk of further
   overflow, and still only affect one or two users, those who are the real culprits.

2) How many users may we warn? One (>50%), 2 (>40) or 3 (>30%)? Certainly not more.
 

A:
---
 1) When a receiving socket is approaching overflow, e.g. at LOW_IMPORTANCE-level*0.75, it 
    scans through the receive queue and identifies if there are senders who are using 
    LOW_IMPORTANCE and have consumed more than e.g. 40% of the buffer size. If there are any,
    (there can't be more than 2), those are added to a "warned-senders" list.

 2) Those senders are sent a unicast message of a new type, called OVERFLOW_WARNING.

 3) When a sender socket receives OVERFLOW_WARNING, it sets a local OVERFLOW_WARNING flag 
    and returns -EAGAIN on all sendmsg() calls for the given port name until it receives 
    an OVERFLOW_FINIS

Re: [RFC PATCH net-next 0/5] introduce linkcongestion control algorithm

Ying Xue — 2013-05-23T01:45:28

Maybe you misunderstood my idea :)

I just listed the truth that TCP/SCTP congestion flow sets snd_ssthresh
to a very high value. But for TIPC, I still prefer to use rx_pending as
initial snd_ssthresh value.

But I think it's worth investigating/verifying the two both cases in
real environment. So I will test them. Later I will give their test results.


Please wait, I will verify the two different cases. If you have other
good idea, please tell me.

Regards,
Ying



------------------------------------------------------------------------------
Try New Relic Now & We'll Send You this Cool Shirt
New Relic is the only SaaS-based application performance monitoring service 
that delivers powerful full stack analytics. Optimize and monitor your
browser, app, & servers with just a few lines of code. Try New Relic
and get this awesome Nerd Life shirt! http://p.sf.net/sfu/newrelic_d2d_may

RDM and multicast flow control

Jon Maloy — 2013-05-22T16:53:20

Erik and I have been discussing how we can find a satisfactory solution to the RDM and 
multicast message overload problem.

I now suggest a combination of two related mechanisms, partially based on Erik's original
suggestion to distribute "overload flags" to all distributed publication items when
overload happens.

A:
---
 1) When a receiving socket is approaching overflow, e.g. at LOW_IMPORTANCE-level*0.75, it 
    scans through the receive queue and identifies the senders which are using LOW_IMPORTANCE. 
 2) Those senders, or if there are too many, those who contribute most of the load (say, 
    representing 50% of the load combined) will be sent a message of a new type, called 
    OVERLOAD_WARNING.
 3) When a sender socket receives OVERLOAD_WARNING, it has to mark itself as OVERLOADED 
    and return -EAGAIN on all sendmsg() calls for the given port name until it receives 
    an OVERLOAD_FINISHED message from the same socket.
 4) The receiving socket issues an OVERLOAD_FINISHED to all the warned sende

Re: [RFC PATCH net-next 0/5] introduce linkcongestion control algorithm

Jon Maloy — 2013-05-22T13:26:09

[...]
.

Ok. I have no doubt this will work. But won´t we need fewer loops, and 
have fewer initial congestion situations, if we start with a value which we know 
is the real upper limit anyway?
If you are certain that it makes no difference if we user 8191 or rx_pending,
we get the same frequency of congestion situations and same performance anyway, 
then I can buy that. You measured both cases, I assume?

My reaction was more to your claim that it is wrong, in principle, to use 
rx_pending as start value. It isn't, as long as we don't build it into the
generic part of the link layer (link.c).

If it really is useful, or if we can do just as well without using it, 
that is a different question which you seem to have answered.

Regards
///jon



------------------------------------------------------------------------------
Try New Relic Now & We'll Send You this Cool Shirt
New Relic is the only SaaS-based application performance monitoring service 
that delivers powerful full stack analytics. Optimize and

Re: [PATCH 2/2] tipc: add ip/udp media type

Ying Xue — 2013-05-22T07:12:08

Anyway, my meanings is that I hope that we should handle upd bearer
errors like recv_notification() of Ethernet bearer.

Regards,
Ying



------------------------------------------------------------------------------
Try New Relic Now & We'll Send You this Cool Shirt
New Relic is the only SaaS-based application performance monitoring service 
that delivers powerful full stack analytics. Optimize and monitor your
browser, app, & servers with just a few lines of code. Try New Relic
and get this awesome Nerd Life shirt! http://p.sf.net/sfu/newrelic_d2d_may

Re: [PATCH 2/2] tipc: add ip/udp media type

Ying Xue — 2013-05-22T07:06:11

But I think if we detect some fatal error is unrecoverable, we not only
print warning message to user,but also we should block the bearer, and
this might be better.

Regards,
Ying



------------------------------------------------------------------------------
Try New Relic Now & We'll Send You this Cool Shirt
New Relic is the only SaaS-based application performance monitoring service 
that delivers powerful full stack analytics. Optimize and monitor your
browser, app, & servers with just a few lines of code. Try New Relic
and get this awesome Nerd Life shirt! http://p.sf.net/sfu/newrelic_d2d_may

Re: [PATCH 2/2] tipc: add ip/udp media type

Erik Hugne — 2013-05-22T06:58:42

I think we only need to override this if we want to handle the network
errors. But there is not much more we can do other than do a helpful 
pr_info/warn/err.

//E

------------------------------------------------------------------------------
Try New Relic Now & We'll Send You this Cool Shirt
New Relic is the only SaaS-based application performance monitoring service 
that delivers powerful full stack analytics. Optimize and monitor your
browser, app, & servers with just a few lines of code. Try New Relic
and get this awesome Nerd Life shirt! http://p.sf.net/sfu/newrelic_d2d_may

Re: [PATCH 2/2] tipc: add ip/udp media type

Ying Xue — 2013-05-22T06:35:08


Additionally I guess we have to override sk_error_report callback
handler to monitor/handle some unrecoverable errors from underlaying udp
bearer like:

http://lxr.free-electrons.com/source/net/rxrpc/ar-error.c#L131

Regards,
Ying


------------------------------------------------------------------------------
Try New Relic Now & We'll Send You this Cool Shirt
New Relic is the only SaaS-based application performance monitoring service 
that delivers powerful full stack analytics. Optimize and monitor your
browser, app, & servers with just a few lines of code. Try New Relic
and get this awesome Nerd Life shirt! http://p.sf.net/sfu/newrelic_d2d_may

Re: [PATCH 2/2] tipc: add ip/udp media type

Ying Xue — 2013-05-22T06:18:13

Sorry, maybe I cannot clearly express my meanings.

In fact, I suggest we should add a new TIPC_MEDIA_UDP in TIPC Kconfig
file so that we can enable or disable the ddp_bearer new feature by our
requirement. For example, InfiniBand media type, it adds a new
TIPC_MEDIA_IB option.

If you think it's unnecessary, it's ok for me.


Fine. :)

Regards,
Ying



------------------------------------------------------------------------------
Try New Relic Now & We'll Send You this Cool Shirt
New Relic is the only SaaS-based application performance monitoring service 
that delivers powerful full stack analytics. Optimize and monitor your
browser, app, & servers with just a few lines of code. Try New Relic
and get this awesome Nerd Life shirt! http://p.sf.net/sfu/newrelic_d2d_may

Re: [PATCH 2/2] tipc: add ip/udp media type

Erik Hugne — 2013-05-22T06:05:02

Not sure i agree with this, we dont have any extra dependencies 
(TIPC already requires CONFIG_INET). And the memory footprint 
for having it enabled is increased by a very small amount.
Jon. do you have any opinion on this?

Ying:
I've fixed it according to your other comments regarding the potential memleak 
in send_msg and useless parameter


------------------------------------------------------------------------------
Try New Relic Now & We'll Send You this Cool Shirt
New Relic is the only SaaS-based application performance monitoring service 
that delivers powerful full stack analytics. Optimize and monitor your
browser, app, & servers with just a few lines of code. Try New Relic
and get this awesome Nerd Life shirt! http://p.sf.net/sfu/newrelic_d2d_may

Re: [PATCH 2/2] tipc: add ip/udp media type

Ying Xue — 2013-05-22T02:50:30

In fact, we should add transmit work instance into udp_bearer instead of
udp_skb_parms so that the work is not per packet, instead per socket. If
so, we just initialize the work once when create transmission socket.


You are right, I misunderstood your idea. But for me, when sending each
packet, we need to allocate a new work instance, which is a little
expensive.

In contrast, as I mentioned before, the transmission work is designed to
per-socket. And we add one outbound queue list in udp_bearer structure.
When send_msg() sends skb buffer, the skb is only inserted into the
outbound queue, subsequently call queue_work() to schedule the
transmission work.

When tipc_udp_send() is scheduled, it should iterate the outbound queue
to send all skb buffers until the queue is empty or something else.

This design seems more elegant.

In fact, once we can see how to implement tipc server in
net/tipc/server.c, you might understand what I say.


It seems better than before.


Yes, list is better than array, and I wil

Re: [PATCH 2/2] tipc: add ip/udp media type

Ying Xue — 2013-05-22T02:04:04

We should add one new config option in Kconfig, for example,
TIPC_MEDIA_UDP, having us not to compile the udp_bearer bearer.

On 05/17/2013 05:45 PM, erik.hugne< at >ericsson.com wrote:

It's unnecessary for us to define the remote variable because we can
directly use dest to memcpy to parms->dst later.


We cannot consolidate two condition statements into one together.
Instead, we should do split them to avoid memory leak as belows:

if (!clone)
return -ENOMEM;
parms = kmalloc(sizeof(struct udp_skb_parms), GFP_ATOMIC);
if (!parms) {
kfree_skb(clone);
return -ENOMEM;
}



------------------------------------------------------------------------------
Try New Relic Now & We'll Send You this Cool Shirt
New Relic is the only SaaS-based application performance monitoring service 
that delivers powerful full stack analytics. Optimize and monitor your
browser, app, & servers with just a few lines of code. Try New Relic
and get this awesome Nerd Life shirt! http://p.sf.net/sfu/newrelic_d2d_may

Re: [PATCH 2/2] tipc: add ip/udp media type

Erik Hugne — 2013-05-21T13:35:57

Thanks Ying!
Responses/questions for all comments inlined below.

On Tue, May 21, 2013 at 06:54:42PM +0800, Ying Xue wrote:

Fixed.


This is needed because the deferred send work might try to push data
out on the socket while the bearer is being disabled.


We need to initialize it here because the work_struct is dynamically allocated 
(udp_skb_parms) per-packet in the send_msg.


queue_work only returns false if the work had already been put on a WQ.
We allocate a work_struct for each packet to be sent, and enqueue it only once.
Even if we get our wires crossed or some weird scheduling bug causes the work
to be enqueued twice, i dont think memory leaks can happen here.


I think this was done in the early versions, but changed to use
(Concurrency Managed) workqueues instead.
Or maybe i'm misunderstanding you?


Fixed.


Yes, it should be safe to remove this check.
But the enabled atomic_t is still neded in the udp bearer struct
to protect the TX path.


Fixed


Fixed.


Fixed.


Fixed.


Fixed.
If we run a

Re: [PATCH 2/2] tipc: add ip/udp media type

Ying Xue — 2013-05-21T10:54:42

It seems unnecessary to define dup_started as atomic variable.


I think the condtion seem unnecessary for us.


Why do we need to initialize the work here? I think we should do the
initialization in enable_bearer().


No, here we should identify queue_work() return value, if failure, we
should release parms as well as the skb, otherwise, memory leak happens.

There has another issues, and skb might be dropped. In fact we should
first insert the skb into one queue buffer list, and schedule send_wq.
In tipc_udp_send(), we should iterate the buffer list to send all skb
buffers in one time or more times.


As we overide sk_data_ready with tipc_udp_recv, so we should with
sk_callback_lock read lock to protect below area.


above statement is unnecessary for us as long as we hold read lock of
sk_callback_lock.


It seems we should remove the white space between ")" and "&".


No, when override socket callback functions, we must hold
sk_callback_lock write lock.


Please notice comments style, we should add comme

Re: [RFC PATCH net-next 0/5] introduce linkcongestion control algorithm

Ying Xue — 2013-05-21T02:30:57

If I understood your question correctly, I guess you meaning is that
receiver side should advertise a lower rwnd to sender if it detects
packet gap or packet disordered.

I think it seems unnecessary for receiver to advertise its rwnd to
sender, because:

1. Currently sender is able to extract packet sequence gap from STATE
message sent by receiver. So, sender can use the gap as one of
conditions of identifying whether congestion occurs or not.

2. Even if receiver is allowed to advertise a lower rwnd to sender when
its received packet is out of order, we cannot find one available field
to convey the "rwnd" info in message header of payload message or link
protocol message.

However, your question reminds me that I recall another idea about how
receiver limits the speed send sends message with your mentioned info:

We can adjust ack policy on receiver side to lower send speed, for
example, delay to send ACK message to receiver.

Regards,
Ying




-------------------------------------------------------------

Re: [RFC PATCH net-next 0/5] introduce linkcongestion control algorithm

Ying Xue — 2013-05-21T02:17:01

Sorry, I cannot understand why it's same as its peer Ethernet receive
ring buffer size. I try to get an exact answer by digging more related
code from Ethernet driver and net dev code(ie. dev.c) to TIPC link
layer. But, at least, for me the link receive window seems different
with its peer Ethernet receive queue size.

Please see below call paths when TIPC link receives packets from driver:

XX_Ethernet_rx_interrupt()
   netif_rx()  <== if the driver doesn't support NAPI, otherwise
   netif_receive_skb() <== if driver supports NAPI
     enqueue_to_backlog()
       napi_schedule()
         raise_softirq_irqoff(NET_RX_SOFTIRQ)
            net_rx_action()
              process_backlog()(ie, n->poll <==> backlog.poll)
                __netif_receive_skb()
                   __netif_receive_skb_core()
                     deliver_skb()
                         tipc_recv_msg()

The rx_pending in struct ethtool_ringparam only affects
XX_Ethernet_rx_interrupt().

For Ethernet driver, there has two type driver: sing

Re: TIPC 1.7.7 question.

2013-05-17T11:02:17

Yes


------------------------------------------------------------------------------
AlienVault Unified Security Management (USM) platform delivers complete
security visibility with the essential security capabilities. Easily and
efficiently configure, manage, and operate all of your security controls
from a single console and one unified framework. Download a free trial.
http://p.sf.net/sfu/alienvault_d2d

Re: TIPC 1.7.7 question.

Azeez Mohammad — 2013-05-17T09:56:04

Hi Eric,

I have WR's latest RCPL 16.

OS has this patch in full
944841a62   net: backlog functions rename

And about this patch
84a326871   net: add limit for socket backlog

The OS has the 'len' but does not have the 'limit'.

    226         struct {
    227                 struct sk_buff *head;
    228                 struct sk_buff *tail;
    229                 int len;
    230         } sk_backlog;

And the OS does not have  sk_add_backlog_limited

[root< at >platbuild linux]# grep -i sk_add_backlog_limited include/net/sock.h
[root< at >platbuild linux]#


With the OS in this situation, is it safe for me to add this condition in tipc_socket.c ?
if (sk_add_backlog(sk, buf))
   res = TIPC_ERR_OVERLOAD;
else
   res = TIPC_OK;

Thanks for taking the time to investigate this.
Azeez

The current code:

226         struct {
227                 struct sk_buff *head;
228                 struct sk_buff *tail;
229                 int len;
230         } sk_backlog;


481 /* OOB backlog add */
482 static inline void __sk_ad

[PATCH 2/2] tipc: add ip/udp media type

2013-05-17T09:45:38

From: Erik Hugne <erik.hugne< at >ericsson.com>

The ip/udp bearer can be configured in a point-to-point
mode by specifying a remote ip/hostname:
tipc-config -be=udp:eth0:<remip>[:<remport>]
If not specified, the UDP port used is 6118 (iana assigned).
Or, it can be enabled in multicast mode:
tipc-config -be=udp:eth0
In this mode, links will be established to all tipc nodes
that are member in the multicast group.

Signed-off-by: Erik Hugne <erik.hugne< at >ericsson.com>
---
 net/tipc/Makefile    |    2 +-
 net/tipc/bearer.h    |    3 +
 net/tipc/core.c      |    4 +
 net/tipc/udp_media.c |  477 ++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 485 insertions(+), 1 deletions(-)
 create mode 100644 net/tipc/udp_media.c

diff --git a/net/tipc/Makefile b/net/tipc/Makefile
index 4df8e02..81fb8bc 100644
--- a/net/tipc/Makefile
+++ b/net/tipc/Makefile
< at >< at > -8,6 +8,6 < at >< at > tipc-y+= addr.o bcast.o bearer.o config.o \
    core.o handler.o link.o discover.o msg.o  \
    name_distr.o  subscr.o name_table.o net.o

[PATCH 0/2] Add IP/UDP bearer support

2013-05-17T09:45:36

From: Erik Hugne <erik.hugne< at >ericsson.com>

I've had this laying around for quite some time now and i figured
it's time to do a push.
The functionality is fairly isolated, the changes done in the core parts and
userspace API is to allow for longer link names that contains a ':' character.
IPv6 support is not yet possible, as that would violate the current protocol
spec (bearer level originating address field is too small).

An updated tipcutils required to configure UDP bearers can be found here:
git clone -b udp_bearer git< at >github.com:Hugne/tipc-config-dev.git

Erik Hugne (2):
  tipc: allow longer link names containing ':' character
  tipc: add ip/udp media type

 include/uapi/linux/tipc_config.h |    9 +-
 net/tipc/Makefile                |    2 +-
 net/tipc/bearer.h                |    3 +
 net/tipc/core.c                  |    4 +
 net/tipc/link.c                  |    5 +-
 net/tipc/udp_media.c             |  477 ++++++++++++++++++++++++++++++++++++++
 6 files changed, 494 insertions(+), 6 deletions(-)