[ovs-dev] [PATCH 3/9] tunneling: Fix location of GRE checksums.

Jesse Gross jesse at nicira.com
Wed Apr 1 00:26:19 UTC 2015


On Tue, Mar 31, 2015 at 4:21 PM, Ben Pfaff <blp at 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 <jesse at 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);



More information about the dev mailing list