| Message ID | 20200509091141.10619-1-pablo@netfilter.org |
|---|---|
| State | Changes Requested |
| Delegated to: | Pablo Neira |
| Headers | show |
| Series | [libnetfilter_queue,1/2] pktbuff: add __pktb_setup() | expand |
On Sat, May 09, 2020 at 11:11:40AM +0200, Pablo Neira Ayuso wrote: > Add private helper function to set up the pkt_buff object. > > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> > --- > src/extra/pktbuff.c | 54 +++++++++++++++++++++++++++------------------ > 1 file changed, 32 insertions(+), 22 deletions(-) > > diff --git a/src/extra/pktbuff.c b/src/extra/pktbuff.c > index 6dd0ca98aff2..118ad898f63b 100644 > --- a/src/extra/pktbuff.c > +++ b/src/extra/pktbuff.c > @@ -29,6 +29,34 @@ > * @{ > */ > > +static int __pktb_setup(int family, struct pkt_buff *pktb) Apart from breakage, this is pktb_setup_family() as I posted in https://www.spinics.net/lists/netfilter-devel/msg66710.html, with args reversed. I also added pktb_setup_metadata() to capture 3 other duplicate code lines, you may not think that worth doing: personal choice I guess. > +{ > + struct ethhdr *ethhdr; > + > + switch (family) { > + case AF_INET: > + case AF_INET6: > + pktb->network_header = pktb->data; > + break; > + case AF_BRIDGE: > + ethhdr = (struct ethhdr *)pktb->data; > + pktb->mac_header = pktb->data; > + > + switch(ethhdr->h_proto) { > + case ETH_P_IP: > + case ETH_P_IPV6: > + pktb->network_header = pktb->data + ETH_HLEN; > + break; > + default: > + /* This protocol is unsupported. */ Should have moved 'errno = EPROTONOSUPPORT;' here rather than leaving it in pktb_alloc(). As things stand, pktb_setup() will leave errno unaltered when this error occurs. > + return -1; > + } > + break; > + } > + > + return 0; > +} > + > /** > * pktb_alloc - allocate a new packet buffer > * \param family Indicate what family. Currently supported families are > @@ -52,7 +80,6 @@ EXPORT_SYMBOL > struct pkt_buff *pktb_alloc(int family, void *data, size_t len, size_t extra) > { > struct pkt_buff *pktb; > - struct ethhdr *ethhdr; > void *pkt_data; > > pktb = calloc(1, sizeof(struct pkt_buff) + len + extra); > @@ -68,28 +95,11 @@ struct pkt_buff *pktb_alloc(int family, void *data, size_t len, size_t extra) > > pktb->data = pkt_data; > > - switch(family) { > - case AF_INET: > - case AF_INET6: > - pktb->network_header = pktb->data; > - break; > - case AF_BRIDGE: > - ethhdr = (struct ethhdr *)pktb->data; > - pktb->mac_header = pktb->data; > - > - switch(ethhdr->h_proto) { > - case ETH_P_IP: > - case ETH_P_IPV6: > - pktb->network_header = pktb->data + ETH_HLEN; > - break; > - default: > - /* This protocol is unsupported. */ > - errno = EPROTONOSUPPORT; > - free(pktb); > - return NULL; > - } > - break; > + if (__pktb_setup(family, pktb) < 0) { > + errno = EPROTONOSUPPORT; As before, above line belongs in __pktb_setup() > + free(pktb); You need 'return NULL;' here, as in the original code. > } > + > return pktb; > } > > -- > 2.20.1 >
diff --git a/src/extra/pktbuff.c b/src/extra/pktbuff.c index 6dd0ca98aff2..118ad898f63b 100644 --- a/src/extra/pktbuff.c +++ b/src/extra/pktbuff.c @@ -29,6 +29,34 @@ * @{ */ +static int __pktb_setup(int family, struct pkt_buff *pktb) +{ + struct ethhdr *ethhdr; + + switch (family) { + case AF_INET: + case AF_INET6: + pktb->network_header = pktb->data; + break; + case AF_BRIDGE: + ethhdr = (struct ethhdr *)pktb->data; + pktb->mac_header = pktb->data; + + switch(ethhdr->h_proto) { + case ETH_P_IP: + case ETH_P_IPV6: + pktb->network_header = pktb->data + ETH_HLEN; + break; + default: + /* This protocol is unsupported. */ + return -1; + } + break; + } + + return 0; +} + /** * pktb_alloc - allocate a new packet buffer * \param family Indicate what family. Currently supported families are @@ -52,7 +80,6 @@ EXPORT_SYMBOL struct pkt_buff *pktb_alloc(int family, void *data, size_t len, size_t extra) { struct pkt_buff *pktb; - struct ethhdr *ethhdr; void *pkt_data; pktb = calloc(1, sizeof(struct pkt_buff) + len + extra); @@ -68,28 +95,11 @@ struct pkt_buff *pktb_alloc(int family, void *data, size_t len, size_t extra) pktb->data = pkt_data; - switch(family) { - case AF_INET: - case AF_INET6: - pktb->network_header = pktb->data; - break; - case AF_BRIDGE: - ethhdr = (struct ethhdr *)pktb->data; - pktb->mac_header = pktb->data; - - switch(ethhdr->h_proto) { - case ETH_P_IP: - case ETH_P_IPV6: - pktb->network_header = pktb->data + ETH_HLEN; - break; - default: - /* This protocol is unsupported. */ - errno = EPROTONOSUPPORT; - free(pktb); - return NULL; - } - break; + if (__pktb_setup(family, pktb) < 0) { + errno = EPROTONOSUPPORT; + free(pktb); } + return pktb; }
Add private helper function to set up the pkt_buff object. Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> --- src/extra/pktbuff.c | 54 +++++++++++++++++++++++++++------------------ 1 file changed, 32 insertions(+), 22 deletions(-)