From: Duncan Roe <duncan_roe@optusnet.com.au>
To: netfilter-devel@vger.kernel.org
Subject: Re: [PATCH libnetfilter_queue 1/2] pktbuff: add __pktb_setup()
Date: Wed, 13 May 2020 16:41:56 +1000 [thread overview]
Message-ID: <20200513064156.GA23132@dimstar.local.net> (raw)
In-Reply-To: <20200509091141.10619-1-pablo@netfilter.org>
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
>
prev parent reply other threads:[~2020-05-13 7:00 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-09 9:11 [PATCH libnetfilter_queue 1/2] pktbuff: add __pktb_setup() Pablo Neira Ayuso
2020-05-09 9:11 ` [PATCH libnetfilter_queue 2/2] pktbuff: add pktb_head_alloc(), pktb_setup() and pktb_head_size() Pablo Neira Ayuso
2020-05-09 16:09 ` Duncan Roe
2020-05-09 17:26 ` Pablo Neira Ayuso
2020-05-20 5:54 ` Duncan Roe
2020-05-13 6:48 ` Duncan Roe
2020-05-13 6:41 ` Duncan Roe [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200513064156.GA23132@dimstar.local.net \
--to=duncan_roe@optusnet.com.au \
--cc=netfilter-devel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).