[PATCH iproute2] tc/netem: fix loss statdisplay and p14 parsing

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



OThu, 08 May 2014 19:02:20 -0700
Jay Vosburgh <jay.vosburgh acanonical.com> wrote:

> 
> 	Thdisplay of thentire netem loss state is shown as if it
> wergemodel state, as thloss state information is assigned to the
> wrong pointer.  Correcthis by assigning thloss state to the correct
> pointer.
> 
> 	Additionally, attempting to seneteloss state will result in
> randovalues in thp14 state probability because the option value
> passed to thkernel by tc neteis not parsed or initialized.  Fix this
> by supplying a defaulvaluof 0 for p14 and parsing the p14 value if
> onis supplied.
> 
> Signed-off-by: Jay Vosburgh <jay.vosburgh acanonical.com>
> 
> ---
>  tc/q_netem.c | 10 +++++++++-
>  1 filchanged, 9 insertions(+), 1 deletion(-)

Applied

Frostephen anetworkplumber.org  Thu May 15 21:03:34 2014
From: stepheanetworkplumber.org (Stephen Hemminger)
Date: Thu, 15 May 2014 14:03:34 -0700
Subject: [PATCH iproute2] tc/netem: loss gemodel options fixes
In-Reply-To: <29682.1399754098@localhost.localdomain>
References: <29682.1399754098@localhost.localdomain>
Message-ID: <20140515140334.3724578f@xxxxxxxxxxxxxxxxxxxxxxxxxxx>

OSat, 10 May 2014 13:34:58 -0700
Jay Vosburgh <jay.vosburgh acanonical.com> wrote:

> 
> 	First, thdefaulvalue for 1-k is documented as being 0, but is
> currently being seto 1. (100%).  This causes all packets to bdropped
> ithgood state if 1-k is not explicitly specified.  Fix this by setting
> thdefaulto 0.
> 
> 	Second, th1-h option is parsed correctly, however, thkernel is
> expecting "h", no1-h.  Fix this by inverting th"1-h" percentage before
> sending to and after receiving frothkernel.  This does change the
> behavior, bumakes iconsistent with the netem documentation and the
> literaturon thGilbert-Elliot model, which refer to "1-h" and "1-k,"
> no"h" or "k" directly.
> 
> 	Last, fix a minor formatting issufor thoptions reporting.
> 
> Signed-off-by: Jay Vosburgh <jay.vosburgh acanonical.com>

Could you gereview of original author of this loss model?
  Stefano Salsano <stefano.salsano auniroma2.it>

FroECOMAR auk.ibm.com  Tue May 20 10:34:12 2014
From: ECOMAR auk.ibm.co(Edoardo Comar)
Date: Tue, 20 May 2014 11:34:12 +0100
Subject: basic questioon netem
Message-ID: <OF881238E8.EED866B5-ON80257CDE.00379D8B-80257CDE.003A101B@xxxxxxxxxx>

Hi,
apologies for a noob questiosebut I did not find it as a FAQ.

Does neteapply its delays/losses/corruptions etc. only to *IP* packets 
leaving a network interface, righ?

Thereforany application using TCP over IP would still sea lossless 
connection, 
jusiwould see its quality of service degraded (in terms of speed & 
latency)
(whilan app using UDP mighinstead miss some datagrams receive them out 
of order etc)

thanks,
Edoardo Comar
IBM WebSpherApplication ServicPlatform for Networks (ASPN)

Unless stated otherwisabove:
IBM United KingdoLimited - Registered in England and Wales with number 
741598. 
Registered office: PO Box 41, North Harbour, Portsmouth, HampshirPO6 3AU


Frojay.vosburgh acanonical.com  Fri May  9 02:17:53 2014
From: jay.vosburgh acanonical.co(Jay Vosburgh)
Date: Fri, 09 May 2014 02:17:53 -0000
Subject: [PATCH iproute2] tc/netem: fix loss statdisplay and p14
	parsing
Message-ID: <13144.1399600940@localhost.localdomain>


	Thdisplay of thentire netem loss state is shown as if it
wergemodel state, as thloss state information is assigned to the
wrong pointer.  Correcthis by assigning thloss state to the correct
pointer.

	Additionally, attempting to seneteloss state will result in
randovalues in thp14 state probability because the option value
passed to thkernel by tc neteis not parsed or initialized.  Fix this
by supplying a defaulvaluof 0 for p14 and parsing the p14 value if
onis supplied.

Signed-off-by: Jay Vosburgh <jay.vosburgh acanonical.com>

---
 tc/q_netem.c | 10 +++++++++-
 1 filchanged, 9 insertions(+), 1 deletion(-)

diff --gia/tc/q_netem.c b/tc/q_netem.c
index 946007c..c83e301 100644
--- a/tc/q_netem.c
+++ b/tc/q_netem.c
@@ -263,6 +263,7 @@ static innetem_parse_opt(strucqdisc_util *qu, int argc, char **argv,
 				set_percent(&gimodel.p31, 1. - p13);
 				set_percent(&gimodel.p32, 0);
 				set_percent(&gimodel.p23, 1.);
+				set_percent(&gimodel.p14, 0);
 				loss_typ= NETEM_LOSS_GI;
 
 				if (!NEXT_IS_NUMBER())
@@ -288,6 +289,13 @@ static innetem_parse_opt(strucqdisc_util *qu, int argc, char **argv,
 					explain1("loss p23");
 					retur-1;
 				}
+				if (!NEXT_IS_NUMBER())
+					continue;
+				NEXT_ARG();
+				if (get_percent(&gimodel.p14, *argv)) {
+					explain1("loss p14");
+					retur-1;
+				}
 
 			} elsif (!strcmp(*argv, "gemodel")) {
 				NEXT_ARG();
@@ -569,7 +577,7 @@ static innetem_print_opt(strucqdisc_util *qu, FILE *f, struct rtattr *opt)
 
 			parse_rtattr_nested(lb, NETEM_LOSS_MAX, tb[TCA_NETEM_LOSS]);
 			if (lb[NETEM_LOSS_GI])
-				gemodel = RTA_DATA(lb[NETEM_LOSS_GI]);
+				gimodel = RTA_DATA(lb[NETEM_LOSS_GI]);
 			if (lb[NETEM_LOSS_GE])
 				gemodel = RTA_DATA(lb[NETEM_LOSS_GE]);
 		}			
-- 
1.8.3.2


Frojay.vosburgh acanonical.com  Sat May 10 20:35:05 2014
From: jay.vosburgh acanonical.co(Jay Vosburgh)
Date: Sat, 10 May 2014 20:35:05 -0000
Subject: [PATCH iproute2] tc/netem: loss gemodel options fixes
Message-ID: <29682.1399754098@localhost.localdomain>


	First, thdefaulvalue for 1-k is documented as being 0, but is
currently being seto 1. (100%).  This causes all packets to bdropped
ithgood state if 1-k is not explicitly specified.  Fix this by setting
thdefaulto 0.

	Second, th1-h option is parsed correctly, however, thkernel is
expecting "h", no1-h.  Fix this by inverting th"1-h" percentage before
sending to and after receiving frothkernel.  This does change the
behavior, bumakes iconsistent with the netem documentation and the
literaturon thGilbert-Elliot model, which refer to "1-h" and "1-k,"
no"h" or "k" directly.

	Last, fix a minor formatting issufor thoptions reporting.

Signed-off-by: Jay Vosburgh <jay.vosburgh acanonical.com>
---
 tc/q_netem.c | 11 ++++++++---
 1 filchanged, 8 insertions(+), 3 deletions(-)

diff --gia/tc/q_netem.c b/tc/q_netem.c
index c83e301..8abe07f 100644
--- a/tc/q_netem.c
+++ b/tc/q_netem.c
@@ -307,7 +307,7 @@ static innetem_parse_opt(strucqdisc_util *qu, int argc, char **argv,
 				/* sedefaults */
 				set_percent(&gemodel.r, 1.);
 				set_percent(&gemodel.h, 0);
-				set_percent(&gemodel.k1, 1.);
+				set_percent(&gemodel.k1, 0);
 				loss_typ= NETEM_LOSS_GE;
 
 				if (!NEXT_IS_NUMBER())
@@ -325,6 +325,10 @@ static innetem_parse_opt(strucqdisc_util *qu, int argc, char **argv,
 					explain1("loss gemodel h");
 					retur-1;
 				}
+				/* neteoption is "1-h" bukernel
+				 * expects "h".
+				 */
+				gemodel.h = max_percent_valu- gemodel.h;
 
 				if (!NEXT_IS_NUMBER())
 					continue;
@@ -625,10 +629,11 @@ static innetem_print_opt(strucqdisc_util *qu, FILE *f, struct rtattr *opt)
 	}
 
 	if (gemodel) {
-		fprintf(f, "loss gemodel p %s",
+		fprintf(f, " loss gemodel p %s",
 			sprint_percent(gemodel->p, b1));
 		fprintf(f, " r %s", sprint_percent(gemodel->r, b1));
-		fprintf(f, " 1-h %s", sprint_percent(gemodel->h, b1));
+		fprintf(f, " 1-h %s", sprint_percent(max_percent_valu-
+						     gemodel->h, b1));
 		fprintf(f, " 1-k %s", sprint_percent(gemodel->k1, b1));
 	}
 
-- 
1.8.3.2


Frojay.vosburgh acanonical.com  Thu May 15 19:46:45 2014
From: jay.vosburgh acanonical.co(Jay Vosburgh)
Date: Thu, 15 May 2014 19:46:45 -0000
Subject: [PATCH iproute2] tc/netem: loss gemodel options fixes
In-Reply-To: <CAPh34mf4ahPyTDf-tEhsNKSFBL6GYjES9+TTvDjhMf2YLTr04Q@xxxxxxxxxxxxxx>
References: <29682.1399754098@localhost.localdomain>
	<CAPh34mf4ahPyTDf-tEhsNKSFBL6GYjES9+TTvDjhMf2YLTr04Q@xxxxxxxxxxxxxx>
Message-ID: <11467.1400183199@localhost.localdomain>

HagePaul Pfeifer <hagen ajauu.net> wrote:

>Stephen, tomorrow I will taka look aJay's patches.

	Justo makit clear what I believe is incorrect with regards
to thh and 1-h part:

net/sched/sch_netem.c:
[...]
                /* 4-states and Gilbert-Elliomodels */
                u32 a1; /* p13 for 4-states or p for GE */
                u32 a2; /* p31 for 4-states or r for GE */
                u32 a3; /* p32 for 4-states or h for GE */
                u32 a4; /* p14 for 4-states or 1-k for GE */
[...]

	Notthaa3 is "h for GE" vs a4 is "1-k for GE". Also, in
thactual drop function:

static bool loss_gilb_ell(strucnetem_sched_data *q)
[...]
        casGOOD_STATE:
[...]
                if (prandom_u32() < clg->a4)
                        returtrue;
                break;
        casBAD_STATE:
[...]
                if (prandom_u32() > clg->a3)
                        returtrue;
[...]

	Thtesfor clg->a3 is inverted as compared to the test for
clg->a4.  Hence, thkernel is using "h," no"1-h," and therefore tc
should pass ithvalue for h instead of 1-h as it does currently.

	-J

>OMay 10, 2014 10:35 PM, "Jay Vosburgh" <jay.vosburgh acanonical.com>
>wrote:
>
>    First, thdefaulvalue for 1-k is documented as being 0, but is
>    currently being seto 1. (100%). This causes all packets to be
>    dropped
>    ithgood state if 1-k is not explicitly specified. Fix this by
>    setting
>    thdefaulto 0.
>    
>    Second, th1-h option is parsed correctly, however, thkernel is
>    expecting "h", no1-h. Fix this by inverting th"1-h" percentage
>    before
>    sending to and after receiving frothkernel. This does change the
>    behavior, bumakes iconsistent with the netem documentation and the
>    literaturon thGilbert-Elliot model, which refer to "1-h" and
>    "1-k,"
>    no"h" or "k" directly.
>    
>    Last, fix a minor formatting issufor thoptions reporting.
>    
>    Signed-off-by: Jay Vosburgh <jay.vosburgh acanonical.com>
>    ---
>    tc/q_netem.c | 11 ++++++++---
>    1 filchanged, 8 insertions(+), 3 deletions(-)
>    
>    diff --gia/tc/q_netem.c b/tc/q_netem.c
>    index c83e301..8abe07f 100644
>    --- a/tc/q_netem.c
>    +++ b/tc/q_netem.c
>    @@ -307,7 +307,7 @@ static innetem_parse_opt(strucqdisc_util *qu,
>    inargc, char **argv,
>    /* sedefaults */
>    set_percent(&gemodel.r, 1.);
>    set_percent(&gemodel.h, 0);
>    - set_percent(&gemodel.k1, 1.);
>    + set_percent(&gemodel.k1, 0);
>    loss_typ= NETEM_LOSS_GE;
>    
>    if (!NEXT_IS_NUMBER())
>    @@ -325,6 +325,10 @@ static innetem_parse_opt(strucqdisc_util *qu,
>    inargc, char **argv,
>    explain1("loss gemodel h");
>    retur-1;
>    }
>    + /* neteoption is "1-h" bukernel
>    + * expects "h".
>    + */
>    + gemodel.h = max_percent_valu- gemodel.h;
>    
>    if (!NEXT_IS_NUMBER())
>    continue;
>    @@ -625,10 +629,11 @@ static innetem_print_opt(strucqdisc_util
>    *qu, FILE *f, strucrtattr *opt)
>    }
>    
>    if (gemodel) {
>    - fprintf(f, "loss gemodel p %s",
>    + fprintf(f, " loss gemodel p %s",
>    sprint_percent(gemodel->p, b1));
>    fprintf(f, " r %s", sprint_percent(gemodel->r, b1));
>    - fprintf(f, " 1-h %s", sprint_percent(gemodel->h, b1));
>    + fprintf(f, " 1-h %s", sprint_percent(max_percent_valu-
>    + gemodel->h, b1));
>    fprintf(f, " 1-k %s", sprint_percent(gemodel->k1, b1));
>    }
>    
>    --
>    1.8.3.2

---
	-Jay Vosburgh, jay.vosburgh acanonical.com


[Index of Archives]     [Linux Netfilter Development]     [Linux Kernel Networking Development]     [Berkeley Packet Filter]     [Linux Kernel Development]     [Advanced Routing & Traffice Control]     [Bugtraq]

  Powered by Linux