diff mbox

[1/2] Fix drop_err_pkts to correctly discard bad packets

Message ID 1401729001-15729-2-git-send-email-stuart.haslam@arm.com
State Accepted
Commit 26844e575d1c8d841835d7d9caa522bd86222460
Headers show

Commit Message

Stuart Haslam June 2, 2014, 5:10 p.m. UTC
The drop_err_pkts function is intended to discard packets that input
parsing has marked as containing errors (e.g. short frame). It correctly
identifies and frees bad packets but an error in the way pkt_tbl is
updated means bad packet handles aren't actually removed, later use of
the stale packet handle results in a SEGV. The pkt_cnt is decremented
though, so for each bad packet a good packet (within the same burst) is
dropped and leaked.

Signed-off-by: Stuart Haslam <stuart.haslam@arm.com>
---
 test/l2fwd/l2fwd.c                            | 2 +-
 test/packet/odp_example_pktio.c               | 2 +-
 test/packet_netmap/odp_example_pktio_netmap.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

Comments

Anders Roxell June 4, 2014, 8:39 p.m. UTC | #1
On 2014-06-02 18:10, Stuart Haslam wrote:
> The drop_err_pkts function is intended to discard packets that input
> parsing has marked as containing errors (e.g. short frame). It correctly
> identifies and frees bad packets but an error in the way pkt_tbl is
> updated means bad packet handles aren't actually removed, later use of
> the stale packet handle results in a SEGV. The pkt_cnt is decremented
> though, so for each bad packet a good packet (within the same burst) is
> dropped and leaked.
> 
> Signed-off-by: Stuart Haslam <stuart.haslam@arm.com>

Reviewed-by: Anders Roxell <anders.roxell@linaro.org>

-Anders

> ---
>  test/l2fwd/l2fwd.c                            | 2 +-
>  test/packet/odp_example_pktio.c               | 2 +-
>  test/packet_netmap/odp_example_pktio_netmap.c | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/test/l2fwd/l2fwd.c b/test/l2fwd/l2fwd.c
> index fb1b949..49e113d 100644
> --- a/test/l2fwd/l2fwd.c
> +++ b/test/l2fwd/l2fwd.c
> @@ -447,7 +447,7 @@ static int drop_err_pkts(odp_packet_t pkt_tbl[], unsigned len)
>  			odp_packet_free(pkt); /* Drop */
>  			pkt_cnt--;
>  		} else if (odp_unlikely(i != j++)) {
> -			pkt_tbl[j] = pkt;
> +			pkt_tbl[j-1] = pkt;
>  		}
>  	}
>  
> diff --git a/test/packet/odp_example_pktio.c b/test/packet/odp_example_pktio.c
> index 3acb1fb..91aa6d1 100644
> --- a/test/packet/odp_example_pktio.c
> +++ b/test/packet/odp_example_pktio.c
> @@ -411,7 +411,7 @@ static int drop_err_pkts(odp_packet_t pkt_tbl[], unsigned len)
>  			odp_packet_free(pkt); /* Drop */
>  			pkt_cnt--;
>  		} else if (odp_unlikely(i != j++)) {
> -			pkt_tbl[j] = pkt;
> +			pkt_tbl[j-1] = pkt;
>  		}
>  	}
>  
> diff --git a/test/packet_netmap/odp_example_pktio_netmap.c b/test/packet_netmap/odp_example_pktio_netmap.c
> index f50f764..7d33b19 100644
> --- a/test/packet_netmap/odp_example_pktio_netmap.c
> +++ b/test/packet_netmap/odp_example_pktio_netmap.c
> @@ -370,7 +370,7 @@ static int drop_err_pkts(odp_packet_t pkt_tbl[], unsigned len)
>  			odp_packet_free(pkt); /* Drop */
>  			pkt_cnt--;
>  		} else if (odp_unlikely(i != j++)) {
> -			pkt_tbl[j] = pkt;
> +			pkt_tbl[j-1] = pkt;
>  		}
>  	}
>  
> -- 
> 1.9.1
> 
> 
> 
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
diff mbox

Patch

diff --git a/test/l2fwd/l2fwd.c b/test/l2fwd/l2fwd.c
index fb1b949..49e113d 100644
--- a/test/l2fwd/l2fwd.c
+++ b/test/l2fwd/l2fwd.c
@@ -447,7 +447,7 @@  static int drop_err_pkts(odp_packet_t pkt_tbl[], unsigned len)
 			odp_packet_free(pkt); /* Drop */
 			pkt_cnt--;
 		} else if (odp_unlikely(i != j++)) {
-			pkt_tbl[j] = pkt;
+			pkt_tbl[j-1] = pkt;
 		}
 	}
 
diff --git a/test/packet/odp_example_pktio.c b/test/packet/odp_example_pktio.c
index 3acb1fb..91aa6d1 100644
--- a/test/packet/odp_example_pktio.c
+++ b/test/packet/odp_example_pktio.c
@@ -411,7 +411,7 @@  static int drop_err_pkts(odp_packet_t pkt_tbl[], unsigned len)
 			odp_packet_free(pkt); /* Drop */
 			pkt_cnt--;
 		} else if (odp_unlikely(i != j++)) {
-			pkt_tbl[j] = pkt;
+			pkt_tbl[j-1] = pkt;
 		}
 	}
 
diff --git a/test/packet_netmap/odp_example_pktio_netmap.c b/test/packet_netmap/odp_example_pktio_netmap.c
index f50f764..7d33b19 100644
--- a/test/packet_netmap/odp_example_pktio_netmap.c
+++ b/test/packet_netmap/odp_example_pktio_netmap.c
@@ -370,7 +370,7 @@  static int drop_err_pkts(odp_packet_t pkt_tbl[], unsigned len)
 			odp_packet_free(pkt); /* Drop */
 			pkt_cnt--;
 		} else if (odp_unlikely(i != j++)) {
-			pkt_tbl[j] = pkt;
+			pkt_tbl[j-1] = pkt;
 		}
 	}