All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net 1/3] pktgen: should not turn off vlan when we disable svlan
@ 2014-05-15  9:46 Hangbin Liu
  2014-05-15  9:46 ` [PATCH net 2/3] pktgen: give user result when disable vlan/svlan Hangbin Liu
  2014-05-15  9:46 ` [PATCH net 3/3] pktgen: disable IPv6 when we want to use IPv4 Hangbin Liu
  0 siblings, 2 replies; 6+ messages in thread
From: Hangbin Liu @ 2014-05-15  9:46 UTC (permalink / raw)
  To: David Miller; +Cc: Francesco Fondelli, netdev, Hangbin Liu

We will turn off svlan when disable vlan, but should keep vlan on when disable
svlan. Also fix the doc typo.

Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 Documentation/networking/pktgen.txt | 2 +-
 net/core/pktgen.c                   | 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/Documentation/networking/pktgen.txt b/Documentation/networking/pktgen.txt
index 0e30c78..672fa23 100644
--- a/Documentation/networking/pktgen.txt
+++ b/Documentation/networking/pktgen.txt
@@ -150,7 +150,7 @@ Examples:
  pgset "svlan_cfi 0"      set canonical format identifier 0-1 (default 0)
 
  pgset "vlan_id 9999"     > 4095 remove vlan and svlan tags
- pgset "svlan 9999"       > 4095 remove svlan tag
+ pgset "svlan_id 9999"    > 4095 remove svlan tag
 
 
  pgset "tos XX"           set former IPv4 TOS field (e.g. "tos 28" for AF11 no ECN, default 00)
diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 0304f98..dcf367f 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -1627,11 +1627,10 @@ static ssize_t pktgen_if_write(struct file *file,
 			pkt_dev->nr_labels = 0;    /* turn off MPLS */
 			sprintf(pg_result, "OK: svlan_id=%u", pkt_dev->svlan_id);
 		} else {
-			pkt_dev->vlan_id = 0xffff; /* turn off VLAN/SVLAN */
 			pkt_dev->svlan_id = 0xffff;
 
 			if (debug)
-				pr_debug("VLAN/SVLAN turned off\n");
+				pr_debug("SVLAN turned off\n");
 		}
 		return count;
 	}
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH net 2/3] pktgen: give user result when disable vlan/svlan
  2014-05-15  9:46 [PATCH net 1/3] pktgen: should not turn off vlan when we disable svlan Hangbin Liu
@ 2014-05-15  9:46 ` Hangbin Liu
  2014-05-15 10:00   ` Daniel Borkmann
  2014-05-15  9:46 ` [PATCH net 3/3] pktgen: disable IPv6 when we want to use IPv4 Hangbin Liu
  1 sibling, 1 reply; 6+ messages in thread
From: Hangbin Liu @ 2014-05-15  9:46 UTC (permalink / raw)
  To: David Miller; +Cc: Francesco Fondelli, netdev, Hangbin Liu

Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 net/core/pktgen.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index dcf367f..1809bdf 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -1573,8 +1573,7 @@ static ssize_t pktgen_if_write(struct file *file,
 			pkt_dev->vlan_id = 0xffff; /* turn off VLAN/SVLAN */
 			pkt_dev->svlan_id = 0xffff;
 
-			if (debug)
-				pr_debug("VLAN/SVLAN turned off\n");
+			sprintf(pg_result, "OK: VLAN/SVLAN turned off");
 		}
 		return count;
 	}
@@ -1629,8 +1628,7 @@ static ssize_t pktgen_if_write(struct file *file,
 		} else {
 			pkt_dev->svlan_id = 0xffff;
 
-			if (debug)
-				pr_debug("SVLAN turned off\n");
+			sprintf(pg_result, "OK: SVLAN turned off");
 		}
 		return count;
 	}
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH net 3/3] pktgen: disable IPv6 when we want to use IPv4
  2014-05-15  9:46 [PATCH net 1/3] pktgen: should not turn off vlan when we disable svlan Hangbin Liu
  2014-05-15  9:46 ` [PATCH net 2/3] pktgen: give user result when disable vlan/svlan Hangbin Liu
@ 2014-05-15  9:46 ` Hangbin Liu
  1 sibling, 0 replies; 6+ messages in thread
From: Hangbin Liu @ 2014-05-15  9:46 UTC (permalink / raw)
  To: David Miller; +Cc: Francesco Fondelli, netdev, Hangbin Liu

Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 net/core/pktgen.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 1809bdf..fabd17e 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -1264,6 +1264,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		if (len < 0)
 			return len;
 
+		pkt_dev->flags &= ~F_IPV6;
+
 		if (copy_from_user(buf, &user_buffer[i], len))
 			return -EFAULT;
 		buf[len] = 0;
@@ -1284,6 +1286,7 @@ static ssize_t pktgen_if_write(struct file *file,
 		if (len < 0)
 			return len;
 
+		pkt_dev->flags &= ~F_IPV6;
 
 		if (copy_from_user(buf, &user_buffer[i], len))
 			return -EFAULT;
@@ -1395,6 +1398,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		if (len < 0)
 			return len;
 
+		pkt_dev->flags &= ~F_IPV6;
+
 		if (copy_from_user(buf, &user_buffer[i], len))
 			return -EFAULT;
 		buf[len] = 0;
@@ -1415,6 +1420,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		if (len < 0)
 			return len;
 
+		pkt_dev->flags &= ~F_IPV6;
+
 		if (copy_from_user(buf, &user_buffer[i], len))
 			return -EFAULT;
 		buf[len] = 0;
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH net 2/3] pktgen: give user result when disable vlan/svlan
  2014-05-15  9:46 ` [PATCH net 2/3] pktgen: give user result when disable vlan/svlan Hangbin Liu
@ 2014-05-15 10:00   ` Daniel Borkmann
  2014-05-15 11:05     ` Hangbin Liu
  0 siblings, 1 reply; 6+ messages in thread
From: Daniel Borkmann @ 2014-05-15 10:00 UTC (permalink / raw)
  To: Hangbin Liu; +Cc: David Miller, Francesco Fondelli, netdev

On 05/15/2014 11:46 AM, Hangbin Liu wrote:
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---
>   net/core/pktgen.c | 6 ++----
>   1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/net/core/pktgen.c b/net/core/pktgen.c
> index dcf367f..1809bdf 100644
> --- a/net/core/pktgen.c
> +++ b/net/core/pktgen.c
> @@ -1573,8 +1573,7 @@ static ssize_t pktgen_if_write(struct file *file,
>   			pkt_dev->vlan_id = 0xffff; /* turn off VLAN/SVLAN */
>   			pkt_dev->svlan_id = 0xffff;
>
> -			if (debug)
> -				pr_debug("VLAN/SVLAN turned off\n");
> +			sprintf(pg_result, "OK: VLAN/SVLAN turned off");

I think that might break user scripts as pg_result is copied to user
space, and currently only expected to return 'OK: svlan_id=%u' if it
was actually successful. Unfortunately, scripts that might only check
for 'OK' in the string could make wrong assumptions later on.

>   		}
>   		return count;
>   	}
> @@ -1629,8 +1628,7 @@ static ssize_t pktgen_if_write(struct file *file,
>   		} else {
>   			pkt_dev->svlan_id = 0xffff;
>
> -			if (debug)
> -				pr_debug("SVLAN turned off\n");
> +			sprintf(pg_result, "OK: SVLAN turned off");

Ditto.

>   		}
>   		return count;
>   	}
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net 2/3] pktgen: give user result when disable vlan/svlan
  2014-05-15 10:00   ` Daniel Borkmann
@ 2014-05-15 11:05     ` Hangbin Liu
  2014-05-15 14:12       ` Hangbin Liu
  0 siblings, 1 reply; 6+ messages in thread
From: Hangbin Liu @ 2014-05-15 11:05 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: David Miller, Francesco Fondelli, netdev

On Thu, May 15, 2014 at 12:00:31PM +0200, Daniel Borkmann wrote:
> On 05/15/2014 11:46 AM, Hangbin Liu wrote:
> >Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> >---
> >  net/core/pktgen.c | 6 ++----
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> >
> >diff --git a/net/core/pktgen.c b/net/core/pktgen.c
> >index dcf367f..1809bdf 100644
> >--- a/net/core/pktgen.c
> >+++ b/net/core/pktgen.c
> >@@ -1573,8 +1573,7 @@ static ssize_t pktgen_if_write(struct file *file,
> >  			pkt_dev->vlan_id = 0xffff; /* turn off VLAN/SVLAN */
> >  			pkt_dev->svlan_id = 0xffff;
> >
> >-			if (debug)
> >-				pr_debug("VLAN/SVLAN turned off\n");
> >+			sprintf(pg_result, "OK: VLAN/SVLAN turned off");
> 
> I think that might break user scripts as pg_result is copied to user
> space, and currently only expected to return 'OK: svlan_id=%u' if it
> was actually successful. Unfortunately, scripts that might only check
> for 'OK' in the string could make wrong assumptions later on.

But the original behavior will keep the last return value, which may make user
confused. e.g.

# echo vlan_id 10 > /proc/net/pktgen/eno4
# cat /proc/net/pktgen/eno4
Params: count 1000  min_pkt_size: 0  max_pkt_size: 0
     frags: 0  delay: 0  clone_skb: 0  ifname: eno4
     flows: 0 flowlen: 0
     queue_map_min: 0  queue_map_max: 0
     dst_min:   dst_max:
        src_min:   src_max:
     src_mac: 6c:ae:8b:20:7b:cc dst_mac: 00:00:00:00:00:00
     udp_src_min: 9  udp_src_max: 9  udp_dst_min: 9  udp_dst_max: 9
     src_mac_count: 0  dst_mac_count: 0
     vlan_id: 10  vlan_p: 0  vlan_cfi: 0
     Flags:
Current:
     pkts-sofar: 0  errors: 0
     started: 0us  stopped: 0us idle: 0us
     seq_num: 0  cur_dst_mac_offset: 0  cur_src_mac_offset: 0
     cur_saddr: 0.0.0.0  cur_daddr: 0.0.0.0
     cur_udp_dst: 0  cur_udp_src: 0
     cur_queue_map: 0
     flows: 0
Result: OK: vlan_id=10

# echo vlan_id 9999 > /proc/net/pktgen/eno4
# cat /proc/net/pktgen/eno4
Params: count 1000  min_pkt_size: 0  max_pkt_size: 0
     frags: 0  delay: 0  clone_skb: 0  ifname: eno4
     flows: 0 flowlen: 0
     queue_map_min: 0  queue_map_max: 0
     dst_min:   dst_max:
        src_min:   src_max:
     src_mac: 6c:ae:8b:20:7b:cc dst_mac: 00:00:00:00:00:00
     udp_src_min: 9  udp_src_max: 9  udp_dst_min: 9  udp_dst_max: 9
     src_mac_count: 0  dst_mac_count: 0
     Flags:
Current:
     pkts-sofar: 0  errors: 0
     started: 0us  stopped: 0us idle: 0us
     seq_num: 0  cur_dst_mac_offset: 0  cur_src_mac_offset: 0
     cur_saddr: 0.0.0.0  cur_daddr: 0.0.0.0
     cur_udp_dst: 0  cur_udp_src: 0
     cur_queue_map: 0
     flows: 0
Result: OK: vlan_id=10

> 
> >  		}
> >  		return count;
> >  	}
> >@@ -1629,8 +1628,7 @@ static ssize_t pktgen_if_write(struct file *file,
> >  		} else {
> >  			pkt_dev->svlan_id = 0xffff;
> >
> >-			if (debug)
> >-				pr_debug("SVLAN turned off\n");
> >+			sprintf(pg_result, "OK: SVLAN turned off");
> 
> Ditto.
> 
> >  		}
> >  		return count;
> >  	}
> >

-- 

Thanks & Best Regards
Hangbin Liu <liuhangbin@gmail.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net 2/3] pktgen: give user result when disable vlan/svlan
  2014-05-15 11:05     ` Hangbin Liu
@ 2014-05-15 14:12       ` Hangbin Liu
  0 siblings, 0 replies; 6+ messages in thread
From: Hangbin Liu @ 2014-05-15 14:12 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: David Miller, Francesco Fondelli, netdev

On Thu, May 15, 2014 at 07:05:56PM +0800, Hangbin Liu wrote:
> On Thu, May 15, 2014 at 12:00:31PM +0200, Daniel Borkmann wrote:
> > On 05/15/2014 11:46 AM, Hangbin Liu wrote:
> > >Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> > >---
> > >  net/core/pktgen.c | 6 ++----
> > >  1 file changed, 2 insertions(+), 4 deletions(-)
> > >
> > >diff --git a/net/core/pktgen.c b/net/core/pktgen.c
> > >index dcf367f..1809bdf 100644
> > >--- a/net/core/pktgen.c
> > >+++ b/net/core/pktgen.c
> > >@@ -1573,8 +1573,7 @@ static ssize_t pktgen_if_write(struct file *file,
> > >  			pkt_dev->vlan_id = 0xffff; /* turn off VLAN/SVLAN */
> > >  			pkt_dev->svlan_id = 0xffff;
> > >
> > >-			if (debug)
> > >-				pr_debug("VLAN/SVLAN turned off\n");
> > >+			sprintf(pg_result, "OK: VLAN/SVLAN turned off");
> > 
> > I think that might break user scripts as pg_result is copied to user
> > space, and currently only expected to return 'OK: svlan_id=%u' if it
> > was actually successful. Unfortunately, scripts that might only check
> > for 'OK' in the string could make wrong assumptions later on.

or maybe we can return 0xffff when disabled vlan? like

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index fabd17e..64afb91 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -1580,7 +1580,10 @@ static ssize_t pktgen_if_write(struct file *file,
                        pkt_dev->vlan_id = 0xffff; /* turn off VLAN/SVLAN */
                        pkt_dev->svlan_id = 0xffff;

-                       sprintf(pg_result, "OK: VLAN/SVLAN turned off");
+                       sprintf(pg_result, "OK: vlan_id=0x%x", pkt_dev->vlan_id);
+
+                       if (debug)
+                               pr_debug("VLAN/SVLAN turned off\n");
                }
                return count;
        }

But I saw dst_mac, src_mac and clear_counters also didn't use the OK: param=value
format.
sprintf(pg_result, "OK: dstmac %pM", pkt_dev->dst_mac);
sprintf(pg_result, "OK: srcmac %pM", pkt_dev->src_mac);
sprintf(pg_result, "OK: Clearing counters.\n");

We can change src_mac and dst_mac to "srcmac=%pM" easily, but I don't know how
to edit counters. return OK: counters=0 ?

> 
> But the original behavior will keep the last return value, which may make user
> confused. e.g.
> 
> # echo vlan_id 10 > /proc/net/pktgen/eno4
> # cat /proc/net/pktgen/eno4
> Params: count 1000  min_pkt_size: 0  max_pkt_size: 0
>      frags: 0  delay: 0  clone_skb: 0  ifname: eno4
>      flows: 0 flowlen: 0
>      queue_map_min: 0  queue_map_max: 0
>      dst_min:   dst_max:
>         src_min:   src_max:
>      src_mac: 6c:ae:8b:20:7b:cc dst_mac: 00:00:00:00:00:00
>      udp_src_min: 9  udp_src_max: 9  udp_dst_min: 9  udp_dst_max: 9
>      src_mac_count: 0  dst_mac_count: 0
>      vlan_id: 10  vlan_p: 0  vlan_cfi: 0
>      Flags:
> Current:
>      pkts-sofar: 0  errors: 0
>      started: 0us  stopped: 0us idle: 0us
>      seq_num: 0  cur_dst_mac_offset: 0  cur_src_mac_offset: 0
>      cur_saddr: 0.0.0.0  cur_daddr: 0.0.0.0
>      cur_udp_dst: 0  cur_udp_src: 0
>      cur_queue_map: 0
>      flows: 0
> Result: OK: vlan_id=10
> 
> # echo vlan_id 9999 > /proc/net/pktgen/eno4
> # cat /proc/net/pktgen/eno4
> Params: count 1000  min_pkt_size: 0  max_pkt_size: 0
>      frags: 0  delay: 0  clone_skb: 0  ifname: eno4
>      flows: 0 flowlen: 0
>      queue_map_min: 0  queue_map_max: 0
>      dst_min:   dst_max:
>         src_min:   src_max:
>      src_mac: 6c:ae:8b:20:7b:cc dst_mac: 00:00:00:00:00:00
>      udp_src_min: 9  udp_src_max: 9  udp_dst_min: 9  udp_dst_max: 9
>      src_mac_count: 0  dst_mac_count: 0
>      Flags:
> Current:
>      pkts-sofar: 0  errors: 0
>      started: 0us  stopped: 0us idle: 0us
>      seq_num: 0  cur_dst_mac_offset: 0  cur_src_mac_offset: 0
>      cur_saddr: 0.0.0.0  cur_daddr: 0.0.0.0
>      cur_udp_dst: 0  cur_udp_src: 0
>      cur_queue_map: 0
>      flows: 0
> Result: OK: vlan_id=10
> 
> > 
> > >  		}
> > >  		return count;
> > >  	}
> > >@@ -1629,8 +1628,7 @@ static ssize_t pktgen_if_write(struct file *file,
> > >  		} else {
> > >  			pkt_dev->svlan_id = 0xffff;
> > >
> > >-			if (debug)
> > >-				pr_debug("SVLAN turned off\n");
> > >+			sprintf(pg_result, "OK: SVLAN turned off");
> > 
> > Ditto.
> > 
> > >  		}
> > >  		return count;
> > >  	}
> > >
> 
> -- 
> 
> Thanks & Best Regards
> Hangbin Liu <liuhangbin@gmail.com>

-- 

Thanks & Best Regards
Hangbin Liu <liuhangbin@gmail.com>

^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2014-05-15 14:12 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-05-15  9:46 [PATCH net 1/3] pktgen: should not turn off vlan when we disable svlan Hangbin Liu
2014-05-15  9:46 ` [PATCH net 2/3] pktgen: give user result when disable vlan/svlan Hangbin Liu
2014-05-15 10:00   ` Daniel Borkmann
2014-05-15 11:05     ` Hangbin Liu
2014-05-15 14:12       ` Hangbin Liu
2014-05-15  9:46 ` [PATCH net 3/3] pktgen: disable IPv6 when we want to use IPv4 Hangbin Liu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.