> On 3 Mar 2017, at 18:52 PM, Philippe Mathieu-Daudé <f4...@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 <dmi...@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é <f4...@amsat.org <mailto:f4...@amsat.org>>
>
> Phil.
