On Thu, Feb 02, 2017 at 03:27:17PM +0100, Andreas Schultz wrote:

----- On Feb 2, 2017, at 3:19 PM, pablo pablo@netfilter.org wrote:

quoted

On Mon, Jan 30, 2017 at 05:37:10PM +0100, Andreas Schultz wrote:

quoted

This unifies duplicate code into a helper. It also prepares the
groundwork to add a lookup version that uses the socket to find
attache pdp contexts.

Signed-off-by: Andreas Schultz <redacted>
---
 drivers/net/gtp.c | 120 +++++++++++++++++++++++-------------------------------
 1 file changed, 51 insertions(+), 69 deletions(-)

diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index c96c71f..6b7a3c2 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c

[...]

quoted

+static struct pdp_ctx *gtp_genl_find_pdp(struct sk_buff *skb,
+					 struct genl_info *info)
+{
+	struct pdp_ctx *pctx;
+
+	if (info->attrs[GTPA_LINK])
+		pctx = gtp_genl_find_pdp_by_link(skb, info);
+	else
+		pctx = ERR_PTR(-EINVAL);
+	if (!pctx)
+		pctx = ERR_PTR(-ENOENT);
+
+	return pctx;
+}

For gtp_genl_find_pdp(),  I think this is easier to read:

       if (!info->attrs[GTPA_LINK])
               return ERR_PTR(-EINVAL);

       pctx = gtp_genl_find_pdp_by_link(skb, info);
if (!pctx)
	return ERR_PTR(-ENOENT);

        return pctx;

Yes, but a later patch (will be submitted after this series is
accepted) will change that to:

        if (info->attrs[GTPA_LINK])
                pctx = gtp_genl_find_pdp_by_link(skb, info);
        else if (info->attrs[GTPA_FD])
                pctx = gtp_genl_find_pdp_by_socket(skb, info);
        else
                pctx = ERR_PTR(-EINVAL);

        if (!pctx)
                pctx = ERR_PTR(-ENOENT);

        return pctx;

I can use your form for this change, but have a larger change
later. Which way do you prefer it?

I see, then leave this as it is.