diff mbox

[1/5] eth: Extend vlan stripping functions

Message ID 1487248176-29602-2-git-send-email-dmitry@daynix.com
State New
Headers show

Commit Message

Dmitry Fleytman Feb. 16, 2017, 12:29 p.m. UTC 
Make VLAN stripping functions return number of bytes
copied to given Ethernet header buffer.

This information should be used to re-compose
packet IOV after VLAN stripping.

Signed-off-by: Dmitry Fleytman <dmitry@daynix.com>
---
 include/net/eth.h |  4 ++--
 net/eth.c         | 25 ++++++++++++++-----------
 2 files changed, 16 insertions(+), 13 deletions(-)

Comments

Philippe Mathieu-Daudé March 3, 2017, 4:52 p.m. UTC | #1 
On 02/16/2017 09:29 AM, Dmitry Fleytman wrote:
> Make VLAN stripping functions return number of bytes
> copied to given Ethernet header buffer.
>
> This information should be used to re-compose
> packet IOV after VLAN stripping.
>
> Signed-off-by: Dmitry Fleytman <dmitry@daynix.com>
> ---
>  include/net/eth.h |  4 ++--
>  net/eth.c         | 25 ++++++++++++++-----------
>  2 files changed, 16 insertions(+), 13 deletions(-)
>
> diff --git a/include/net/eth.h b/include/net/eth.h
> index 2013175..afeb45b 100644
> --- a/include/net/eth.h
> +++ b/include/net/eth.h
> @@ -331,12 +331,12 @@ eth_get_pkt_tci(const void *p)
>      }
>  }
>
> -bool
> +size_t
>  eth_strip_vlan(const struct iovec *iov, int iovcnt, size_t iovoff,
>                 uint8_t *new_ehdr_buf,
>                 uint16_t *payload_offset, uint16_t *tci);
>
> -bool
> +size_t
>  eth_strip_vlan_ex(const struct iovec *iov, int iovcnt, size_t iovoff,
>                    uint16_t vet, uint8_t *new_ehdr_buf,
>                    uint16_t *payload_offset, uint16_t *tci);
> diff --git a/net/eth.c b/net/eth.c
> index df81efb..5b9ba26 100644
> --- a/net/eth.c
> +++ b/net/eth.c
> @@ -232,7 +232,7 @@ void eth_get_protocols(const struct iovec *iov, int iovcnt,
>      }
>  }
>
> -bool
> +size_t
>  eth_strip_vlan(const struct iovec *iov, int iovcnt, size_t iovoff,
>                 uint8_t *new_ehdr_buf,
>                 uint16_t *payload_offset, uint16_t *tci)
> @@ -244,7 +244,7 @@ eth_strip_vlan(const struct iovec *iov, int iovcnt, size_t iovoff,
>                                 new_ehdr, sizeof(*new_ehdr));
>
>      if (copied < sizeof(*new_ehdr)) {
> -        return false;
> +        return 0;
>      }
>
>      switch (be16_to_cpu(new_ehdr->h_proto)) {
> @@ -254,7 +254,7 @@ eth_strip_vlan(const struct iovec *iov, int iovcnt, size_t iovoff,
>                              &vlan_hdr, sizeof(vlan_hdr));
>
>          if (copied < sizeof(vlan_hdr)) {
> -            return false;
> +            return 0;
>          }
>
>          new_ehdr->h_proto = vlan_hdr.h_proto;
> @@ -268,18 +268,21 @@ eth_strip_vlan(const struct iovec *iov, int iovcnt, size_t iovoff,
>                                  PKT_GET_VLAN_HDR(new_ehdr), sizeof(vlan_hdr));
>
>              if (copied < sizeof(vlan_hdr)) {
> -                return false;
> +                return 0;
>              }
>
>              *payload_offset += sizeof(vlan_hdr);
> +
> +            return sizeof(struct eth_header) + sizeof(struct vlan_header);
> +        } else {
> +            return sizeof(struct eth_header);

remove this "else return ..."

>          }
> -        return true;

here avoid fall-through with 'return sizeof(struct eth_header);'

>      default:
> -        return false;
> +        return 0;
>      }
>  }
>
> -bool
> +size_t
>  eth_strip_vlan_ex(const struct iovec *iov, int iovcnt, size_t iovoff,
>                    uint16_t vet, uint8_t *new_ehdr_buf,
>                    uint16_t *payload_offset, uint16_t *tci)
> @@ -291,7 +294,7 @@ eth_strip_vlan_ex(const struct iovec *iov, int iovcnt, size_t iovoff,
>                                 new_ehdr, sizeof(*new_ehdr));
>
>      if (copied < sizeof(*new_ehdr)) {
> -        return false;
> +        return 0;
>      }
>
>      if (be16_to_cpu(new_ehdr->h_proto) == vet) {
> @@ -299,17 +302,17 @@ eth_strip_vlan_ex(const struct iovec *iov, int iovcnt, size_t iovoff,
>                              &vlan_hdr, sizeof(vlan_hdr));
>
>          if (copied < sizeof(vlan_hdr)) {
> -            return false;
> +            return 0;
>          }
>
>          new_ehdr->h_proto = vlan_hdr.h_proto;
>
>          *tci = be16_to_cpu(vlan_hdr.h_tci);
>          *payload_offset = iovoff + sizeof(*new_ehdr) + sizeof(vlan_hdr);
> -        return true;
> +        return sizeof(struct eth_header);
>      }
>
> -    return false;
> +    return 0;
>  }
>
>  void
>

with changes:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Phil.
Dmitry Fleytman March 5, 2017, 7:20 a.m. UTC | #2 
> On 3 Mar 2017, at 18:52 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> 
> On 02/16/2017 09:29 AM, Dmitry Fleytman wrote:
>> Make VLAN stripping functions return number of bytes
>> copied to given Ethernet header buffer.
>> 
>> This information should be used to re-compose
>> packet IOV after VLAN stripping.
>> 
>> Signed-off-by: Dmitry Fleytman <dmitry@daynix.com>
>> ---
>> include/net/eth.h |  4 ++--
>> net/eth.c         | 25 ++++++++++++++-----------
>> 2 files changed, 16 insertions(+), 13 deletions(-)
>> 
>> diff --git a/include/net/eth.h b/include/net/eth.h
>> index 2013175..afeb45b 100644
>> --- a/include/net/eth.h
>> +++ b/include/net/eth.h
>> @@ -331,12 +331,12 @@ eth_get_pkt_tci(const void *p)
>>     }
>> }
>> 
>> -bool
>> +size_t
>> eth_strip_vlan(const struct iovec *iov, int iovcnt, size_t iovoff,
>>                uint8_t *new_ehdr_buf,
>>                uint16_t *payload_offset, uint16_t *tci);
>> 
>> -bool
>> +size_t
>> eth_strip_vlan_ex(const struct iovec *iov, int iovcnt, size_t iovoff,
>>                   uint16_t vet, uint8_t *new_ehdr_buf,
>>                   uint16_t *payload_offset, uint16_t *tci);
>> diff --git a/net/eth.c b/net/eth.c
>> index df81efb..5b9ba26 100644
>> --- a/net/eth.c
>> +++ b/net/eth.c
>> @@ -232,7 +232,7 @@ void eth_get_protocols(const struct iovec *iov, int iovcnt,
>>     }
>> }
>> 
>> -bool
>> +size_t
>> eth_strip_vlan(const struct iovec *iov, int iovcnt, size_t iovoff,
>>                uint8_t *new_ehdr_buf,
>>                uint16_t *payload_offset, uint16_t *tci)
>> @@ -244,7 +244,7 @@ eth_strip_vlan(const struct iovec *iov, int iovcnt, size_t iovoff,
>>                                new_ehdr, sizeof(*new_ehdr));
>> 
>>     if (copied < sizeof(*new_ehdr)) {
>> -        return false;
>> +        return 0;
>>     }
>> 
>>     switch (be16_to_cpu(new_ehdr->h_proto)) {
>> @@ -254,7 +254,7 @@ eth_strip_vlan(const struct iovec *iov, int iovcnt, size_t iovoff,
>>                             &vlan_hdr, sizeof(vlan_hdr));
>> 
>>         if (copied < sizeof(vlan_hdr)) {
>> -            return false;
>> +            return 0;
>>         }
>> 
>>         new_ehdr->h_proto = vlan_hdr.h_proto;
>> @@ -268,18 +268,21 @@ eth_strip_vlan(const struct iovec *iov, int iovcnt, size_t iovoff,
>>                                 PKT_GET_VLAN_HDR(new_ehdr), sizeof(vlan_hdr));
>> 
>>             if (copied < sizeof(vlan_hdr)) {
>> -                return false;
>> +                return 0;
>>             }
>> 
>>             *payload_offset += sizeof(vlan_hdr);
>> +
>> +            return sizeof(struct eth_header) + sizeof(struct vlan_header);
>> +        } else {
>> +            return sizeof(struct eth_header);
> 
> remove this "else return ..."
> 
>>         }
>> -        return true;
> 
> here avoid fall-through with 'return sizeof(struct eth_header);’


Hi Philippe,

IIUYC there is no functional difference, just a matter of style. Am I right?

As for me current code is more explicit so I would leave it as is unless
there is a specific project coding style requirement.

~Dmitry

> 
>>     default:
>> -        return false;
>> +        return 0;
>>     }
>> }
>> 
>> -bool
>> +size_t
>> eth_strip_vlan_ex(const struct iovec *iov, int iovcnt, size_t iovoff,
>>                   uint16_t vet, uint8_t *new_ehdr_buf,
>>                   uint16_t *payload_offset, uint16_t *tci)
>> @@ -291,7 +294,7 @@ eth_strip_vlan_ex(const struct iovec *iov, int iovcnt, size_t iovoff,
>>                                new_ehdr, sizeof(*new_ehdr));
>> 
>>     if (copied < sizeof(*new_ehdr)) {
>> -        return false;
>> +        return 0;
>>     }
>> 
>>     if (be16_to_cpu(new_ehdr->h_proto) == vet) {
>> @@ -299,17 +302,17 @@ eth_strip_vlan_ex(const struct iovec *iov, int iovcnt, size_t iovoff,
>>                             &vlan_hdr, sizeof(vlan_hdr));
>> 
>>         if (copied < sizeof(vlan_hdr)) {
>> -            return false;
>> +            return 0;
>>         }
>> 
>>         new_ehdr->h_proto = vlan_hdr.h_proto;
>> 
>>         *tci = be16_to_cpu(vlan_hdr.h_tci);
>>         *payload_offset = iovoff + sizeof(*new_ehdr) + sizeof(vlan_hdr);
>> -        return true;
>> +        return sizeof(struct eth_header);
>>     }
>> 
>> -    return false;
>> +    return 0;
>> }
>> 
>> void
>> 
> 
> with changes:
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org <mailto:f4bug@amsat.org>>
> 
> Phil.
diff mbox

Patch

diff --git a/include/net/eth.h b/include/net/eth.h
index 2013175..afeb45b 100644
--- a/include/net/eth.h
+++ b/include/net/eth.h
@@ -331,12 +331,12 @@  eth_get_pkt_tci(const void *p)
     }
 }
 
-bool
+size_t
 eth_strip_vlan(const struct iovec *iov, int iovcnt, size_t iovoff,
                uint8_t *new_ehdr_buf,
                uint16_t *payload_offset, uint16_t *tci);
 
-bool
+size_t
 eth_strip_vlan_ex(const struct iovec *iov, int iovcnt, size_t iovoff,
                   uint16_t vet, uint8_t *new_ehdr_buf,
                   uint16_t *payload_offset, uint16_t *tci);
diff --git a/net/eth.c b/net/eth.c
index df81efb..5b9ba26 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -232,7 +232,7 @@  void eth_get_protocols(const struct iovec *iov, int iovcnt,
     }
 }
 
-bool
+size_t
 eth_strip_vlan(const struct iovec *iov, int iovcnt, size_t iovoff,
                uint8_t *new_ehdr_buf,
                uint16_t *payload_offset, uint16_t *tci)
@@ -244,7 +244,7 @@  eth_strip_vlan(const struct iovec *iov, int iovcnt, size_t iovoff,
                                new_ehdr, sizeof(*new_ehdr));
 
     if (copied < sizeof(*new_ehdr)) {
-        return false;
+        return 0;
     }
 
     switch (be16_to_cpu(new_ehdr->h_proto)) {
@@ -254,7 +254,7 @@  eth_strip_vlan(const struct iovec *iov, int iovcnt, size_t iovoff,
                             &vlan_hdr, sizeof(vlan_hdr));
 
         if (copied < sizeof(vlan_hdr)) {
-            return false;
+            return 0;
         }
 
         new_ehdr->h_proto = vlan_hdr.h_proto;
@@ -268,18 +268,21 @@  eth_strip_vlan(const struct iovec *iov, int iovcnt, size_t iovoff,
                                 PKT_GET_VLAN_HDR(new_ehdr), sizeof(vlan_hdr));
 
             if (copied < sizeof(vlan_hdr)) {
-                return false;
+                return 0;
             }
 
             *payload_offset += sizeof(vlan_hdr);
+
+            return sizeof(struct eth_header) + sizeof(struct vlan_header);
+        } else {
+            return sizeof(struct eth_header);
         }
-        return true;
     default:
-        return false;
+        return 0;
     }
 }
 
-bool
+size_t
 eth_strip_vlan_ex(const struct iovec *iov, int iovcnt, size_t iovoff,
                   uint16_t vet, uint8_t *new_ehdr_buf,
                   uint16_t *payload_offset, uint16_t *tci)
@@ -291,7 +294,7 @@  eth_strip_vlan_ex(const struct iovec *iov, int iovcnt, size_t iovoff,
                                new_ehdr, sizeof(*new_ehdr));
 
     if (copied < sizeof(*new_ehdr)) {
-        return false;
+        return 0;
     }
 
     if (be16_to_cpu(new_ehdr->h_proto) == vet) {
@@ -299,17 +302,17 @@  eth_strip_vlan_ex(const struct iovec *iov, int iovcnt, size_t iovoff,
                             &vlan_hdr, sizeof(vlan_hdr));
 
         if (copied < sizeof(vlan_hdr)) {
-            return false;
+            return 0;
         }
 
         new_ehdr->h_proto = vlan_hdr.h_proto;
 
         *tci = be16_to_cpu(vlan_hdr.h_tci);
         *payload_offset = iovoff + sizeof(*new_ehdr) + sizeof(vlan_hdr);
-        return true;
+        return sizeof(struct eth_header);
     }
 
-    return false;
+    return 0;
 }
 
 void