On Tue, Mar 31, 2015 at 4:21 PM, Ben Pfaff <b...@nicira.com> wrote:
> On Mon, Mar 30, 2015 at 03:14:46PM -0700, Jesse Gross wrote:
>> The GRE checksum is a 16 bit field stored in a 32 bit option (the
>> rest is reserved). The current code treats the checksum as a 32-bit
>> field and places it in the right place for little endian systems but
>> not big endian. This fixes the problem by storing the 16 bit field
>> directly.
>>
>> Signed-off-by: Jesse Gross <je...@nicira.com>
>> ---
>> lib/netdev-vport.c | 6 ++----
>> lib/odp-util.c | 8 ++++----
>> 2 files changed, 6 insertions(+), 8 deletions(-)
>>
>> diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
>> index 1ee68bc..a9639d3 100644
>> --- a/lib/netdev-vport.c
>> +++ b/lib/netdev-vport.c
>> @@ -996,10 +996,8 @@ netdev_gre_push_header__(struct dp_packet *packet,
>> greh = push_ip_header(packet, header, size, &ip_tot_size);
>>
>> if (greh->flags & htons(GRE_CSUM)) {
>> - ovs_16aligned_be32 *options = (ovs_16aligned_be32 *) (greh + 1);
>> -
>> - put_16aligned_be32(options,
>> - (OVS_FORCE ovs_be32) csum(greh, ip_tot_size -
>> sizeof (struct ip_header)));
>> + ovs_be16 *csum_opt = (ovs_be16 *) (greh + 1);
>> + *csum_opt = csum(greh, ip_tot_size - sizeof (struct ip_header));
>
> Does the above zero the other 16 bits?
This should be OK because the entire header was memset to zero when it
was built at flow creation time.
>> diff --git a/lib/odp-util.c b/lib/odp-util.c
>> index 827b91c..5990edc 100644
>> --- a/lib/odp-util.c
>> +++ b/lib/odp-util.c
>> @@ -565,7 +565,7 @@ format_odp_tnl_push_header(struct ds *ds, struct
>> ovs_action_push_tnl *data)
>> greh->flags, ntohs(greh->protocol));
>> options = (ovs_16aligned_be32 *)(greh + 1);
>> if (greh->flags & htons(GRE_CSUM)) {
>> - ds_put_format(ds, ",csum=0x%"PRIx32,
>> ntohl(get_16aligned_be32(options)));
>> + ds_put_format(ds, ",csum=0x%"PRIx16, ntohs(*((ovs_be16
>> *)options)));
>> options++;
>> }
>> if (greh->flags & htons(GRE_KEY)) {
>> @@ -913,12 +913,12 @@ ovs_parse_tnl_push(const char *s, struct
>> ovs_action_push_tnl *data)
>> ovs_16aligned_be32 *options = (ovs_16aligned_be32 *) (greh + 1);
>>
>> if (greh->flags & htons(GRE_CSUM)) {
>> - uint32_t csum;
>> + uint16_t csum;
>>
>> - if (!ovs_scan_len(s, &n, ",csum=0x%"SCNx32, &csum)) {
>> + if (!ovs_scan_len(s, &n, ",csum=0x%"SCNx16, &csum)) {
>> return -EINVAL;
>> }
>> - put_16aligned_be32(options, htonl(csum));
>> + *((ovs_be16 *)options) = htons(csum);
>> options++;
>
> What about here?
This one isn't zeroed so I so added a memset before the checksum is set:
+ memset(options, 0, sizeof *options);
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
