netdev.vger.kernel.org archive mirror
 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).