diff mbox

[v1] netdev: Make 100 percents packets sampled when sampling rate is 1.

Message ID 1438585918-20233-1-git-send-email-wenyuz@vmware.com
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Wenyu Zhang Aug. 3, 2015, 7:11 a.m. UTC
When sampling rate is 1, the sampling probability is UINT32_MAX. The packet
should be sampled even the prandom32() generate the number of UINT32_MAX.
And none packet need be sampled when the probability is 0.

Signed-off-by: Wenyu Zhang <wenyuz@vmware.com>
---
 net/openvswitch/actions.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Pravin B Shelar Aug. 3, 2015, 6:18 p.m. UTC | #1
On Mon, Aug 3, 2015 at 12:11 AM, Wenyu Zhang <wenyuz@vmware.com> wrote:
> When sampling rate is 1, the sampling probability is UINT32_MAX. The packet
> should be sampled even the prandom32() generate the number of UINT32_MAX.
> And none packet need be sampled when the probability is 0.
>
> Signed-off-by: Wenyu Zhang <wenyuz@vmware.com>
> ---
>  net/openvswitch/actions.c |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
> index cf04c2f..03acb09 100644
> --- a/net/openvswitch/actions.c
> +++ b/net/openvswitch/actions.c
> @@ -669,9 +669,11 @@ static int sample(struct datapath *dp, struct sk_buff *skb,
>
>         for (a = nla_data(attr), rem = nla_len(attr); rem > 0;
>                  a = nla_next(a, &rem)) {
> +               uint32_t probability;
>                 switch (nla_type(a)) {
>                 case OVS_SAMPLE_ATTR_PROBABILITY:
> -                       if (prandom_u32() >= nla_get_u32(a))
> +                       probability = nla_get_u32(a);
> +                       if (!probability || probability > nla_get_u32(a))

This condition does not looks right to calculate sampling probability.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jesse Gross Aug. 3, 2015, 6:30 p.m. UTC | #2
On Mon, Aug 3, 2015 at 11:18 AM, Pravin Shelar <pshelar@nicira.com> wrote:
> On Mon, Aug 3, 2015 at 12:11 AM, Wenyu Zhang <wenyuz@vmware.com> wrote:
>> When sampling rate is 1, the sampling probability is UINT32_MAX. The packet
>> should be sampled even the prandom32() generate the number of UINT32_MAX.
>> And none packet need be sampled when the probability is 0.
>>
>> Signed-off-by: Wenyu Zhang <wenyuz@vmware.com>
>> ---
>>  net/openvswitch/actions.c |    4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
>> index cf04c2f..03acb09 100644
>> --- a/net/openvswitch/actions.c
>> +++ b/net/openvswitch/actions.c
>> @@ -669,9 +669,11 @@ static int sample(struct datapath *dp, struct sk_buff *skb,
>>
>>         for (a = nla_data(attr), rem = nla_len(attr); rem > 0;
>>                  a = nla_next(a, &rem)) {
>> +               uint32_t probability;
>>                 switch (nla_type(a)) {
>>                 case OVS_SAMPLE_ATTR_PROBABILITY:
>> -                       if (prandom_u32() >= nla_get_u32(a))
>> +                       probability = nla_get_u32(a);
>> +                       if (!probability || probability > nla_get_u32(a))
>
> This condition does not looks right to calculate sampling probability.

When you send v2, can you also make the subject more narrow
("openvswitch" instead of "netdev") and add the tree that you are
targeting ("[PATCH net]" in this case)?
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index cf04c2f..03acb09 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -669,9 +669,11 @@  static int sample(struct datapath *dp, struct sk_buff *skb,
 
 	for (a = nla_data(attr), rem = nla_len(attr); rem > 0;
 		 a = nla_next(a, &rem)) {
+		uint32_t probability;
 		switch (nla_type(a)) {
 		case OVS_SAMPLE_ATTR_PROBABILITY:
-			if (prandom_u32() >= nla_get_u32(a))
+			probability = nla_get_u32(a);
+			if (!probability || probability > nla_get_u32(a))
 				return 0;
 			break;