| Message ID | 1452080019-25124-2-git-send-email-stuart.haslam@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Wed, Jan 6, 2016 at 5:33 AM, Stuart Haslam <stuart.haslam@linaro.org> wrote: > The example scripts run_esp_out, run_ah_out and run_both_out are failing > due to test packet meta data being initialised incorrectly. > > Signed-off-by: Stuart Haslam <stuart.haslam@linaro.org> > --- > This is a minimal change to fix the bug, but I'm not convinced the way > packets are created here is valid. It calls odp_packet_alloc() with a > length of 0, then starts writing directly to the odp_packet_data() > pointer. This works with linux-generic because allocating a packet of > length 0 gets a packet with the data length initialised to the length > specified in odp_pool_create(), but this behaviour isn't defined. > That is erroneous behavior. If you allocate a packet of length 0 then odp_packet_len() will report 0 so the code would be writing beyond the end of the packet. Such behavior is, by definition, undefined. If you alloc a packet with length 0 then you need to first call odp_packet_push_tail() to sweep out the number of bytes you want to be able to write. Of course, the RC for this must be checked since the push may or may not be successful. > > example/ipsec/odp_ipsec_stream.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/example/ipsec/odp_ipsec_stream.c > b/example/ipsec/odp_ipsec_stream.c > index f750e18..4d908e4 100644 > --- a/example/ipsec/odp_ipsec_stream.c > +++ b/example/ipsec/odp_ipsec_stream.c > @@ -356,7 +356,7 @@ odp_packet_t create_ipv4_packet(stream_db_entry_t > *stream, > } > > /* Correct set packet length offsets */ > - odp_packet_push_tail(pkt, data - base); > + odp_packet_pull_tail(pkt, odp_packet_len(pkt) - (data - base)); > odp_packet_l2_offset_set(pkt, (uint8_t *)eth - base); > odp_packet_l3_offset_set(pkt, (uint8_t *)ip - base); > odp_packet_l4_offset_set(pkt, ((uint8_t *)ip - base) + > sizeof(*ip)); > -- > 2.1.1 > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/lng-odp >
On Wed, Jan 06, 2016 at 04:10:01PM -0600, Bill Fischofer wrote: > On Wed, Jan 6, 2016 at 5:33 AM, Stuart Haslam <stuart.haslam@linaro.org> > wrote: > > > The example scripts run_esp_out, run_ah_out and run_both_out are failing > > due to test packet meta data being initialised incorrectly. > > > > Signed-off-by: Stuart Haslam <stuart.haslam@linaro.org> > > --- > > This is a minimal change to fix the bug, but I'm not convinced the way > > packets are created here is valid. It calls odp_packet_alloc() with a > > length of 0, then starts writing directly to the odp_packet_data() > > pointer. This works with linux-generic because allocating a packet of > > length 0 gets a packet with the data length initialised to the length > > specified in odp_pool_create(), but this behaviour isn't defined. > > > > That is erroneous behavior. If you allocate a packet of length 0 then > odp_packet_len() will report 0 so the code would be writing beyond the end > of the packet. Such behavior is, by definition, undefined. If you alloc a > packet with length 0 then you need to first call odp_packet_push_tail() to > sweep out the number of bytes you want to be able to write. Of course, the > RC for this must be checked since the push may or may not be successful. That certainly sounds more sensible than what currently happens. I'll resend with an additional patch fixing odp_packet_alloc(), then modify create_ipv4_packet() to incrementally push out the tail before writing to the packet.
diff --git a/example/ipsec/odp_ipsec_stream.c b/example/ipsec/odp_ipsec_stream.c index f750e18..4d908e4 100644 --- a/example/ipsec/odp_ipsec_stream.c +++ b/example/ipsec/odp_ipsec_stream.c @@ -356,7 +356,7 @@ odp_packet_t create_ipv4_packet(stream_db_entry_t *stream, } /* Correct set packet length offsets */ - odp_packet_push_tail(pkt, data - base); + odp_packet_pull_tail(pkt, odp_packet_len(pkt) - (data - base)); odp_packet_l2_offset_set(pkt, (uint8_t *)eth - base); odp_packet_l3_offset_set(pkt, (uint8_t *)ip - base); odp_packet_l4_offset_set(pkt, ((uint8_t *)ip - base) + sizeof(*ip));
The example scripts run_esp_out, run_ah_out and run_both_out are failing due to test packet meta data being initialised incorrectly. Signed-off-by: Stuart Haslam <stuart.haslam@linaro.org> --- This is a minimal change to fix the bug, but I'm not convinced the way packets are created here is valid. It calls odp_packet_alloc() with a length of 0, then starts writing directly to the odp_packet_data() pointer. This works with linux-generic because allocating a packet of length 0 gets a packet with the data length initialised to the length specified in odp_pool_create(), but this behaviour isn't defined. example/ipsec/odp_ipsec_stream.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)