Re: [PATCH net-next V5 2/4] net/dst: Utility functions to build dst_metadata without supplying an skb

On Sun, Sep 4, 2016 at 2:14 PM, Sergei Shtylyov
[off-list ref] wrote:

Hello.


On 9/4/2016 1:55 PM, Hadar Hen Zion wrote:

quoted

From: Amir Vadai <redacted>

Extract __ip_tun_set_dst() and __ipv6_tun_set_dst() out of
ip_tun_rx_dst() and ipv6_tun_rx_dst(), to be used without supplying an
skb.

Signed-off-by: Amir Vadai <redacted>
Signed-off-by: Hadar Hen Zion <redacted>
Acked-by: Jiri Pirko <redacted>
Reviewed-by: Shmulik Ladkani <redacted>
---
 include/net/dst_metadata.h | 45
++++++++++++++++++++++++++++++++-------------
 1 file changed, 32 insertions(+), 13 deletions(-)

diff --git a/include/net/dst_metadata.h b/include/net/dst_metadata.h
index 5db9f59..49e8847 100644
--- a/include/net/dst_metadata.h
+++ b/include/net/dst_metadata.h

@@ -112,12 +112,10 @@ static inline struct ip_tunnel_info

*skb_tunnel_info_unclone(struct sk_buff *skb
        return &dst->u.tun_info;
 }

-static inline struct metadata_dst *ip_tun_rx_dst(struct sk_buff *skb,
-                                                __be16 flags,
-                                                __be64 tunnel_id,
-                                                int md_size)
+static inline struct metadata_dst *
+__ip_tun_set_dst(__be32 saddr, __be32 daddr, __u8 tos, __u8 ttl,
+                __be16 flags, __be64 tunnel_id, int md_size)

   The continuation lines should start under the 1st '__be32' on the broken
up line. See how it was before your patch.

ack

quoted

 {
-       const struct iphdr *iph = ip_hdr(skb);
        struct metadata_dst *tun_dst;

        tun_dst = tun_rx_dst(md_size);

@@ -125,17 +123,27 @@ static inline struct metadata_dst

*ip_tun_rx_dst(struct sk_buff *skb,

[...]

quoted

-static inline struct metadata_dst *ipv6_tun_rx_dst(struct sk_buff *skb,
+static inline struct metadata_dst *ip_tun_rx_dst(struct sk_buff *skb,
                                                 __be16 flags,
                                                 __be64 tunnel_id,
                                                 int md_size)
 {
-       const struct ipv6hdr *ip6h = ipv6_hdr(skb);
+       const struct iphdr *iph = ip_hdr(skb);
+
+       return __ip_tun_set_dst(iph->saddr, iph->daddr, iph->tos,
iph->ttl,
+                               flags, tunnel_id, md_size);
+}
+
+static inline struct metadata_dst *
+__ipv6_tun_set_dst(const struct in6_addr *saddr, const struct in6_addr
*daddr,
+                  __u8 tos, __u8 ttl, __be32 label, __be16 flags,
+                  __be64 tunnel_id, int md_size)

   The continuation lines should start under the 1st *const* on the broken
up line.

ack

quoted

+{
        struct metadata_dst *tun_dst;
        struct ip_tunnel_info *info;

@@ -150,14 +158,25 @@ static inline struct metadata_dst

*ipv6_tun_rx_dst(struct sk_buff *skb,

[...]

quoted

+static inline struct metadata_dst *
+ipv6_tun_rx_dst(struct sk_buff *skb, __be16 flags, __be64 tunnel_id,
+               int md_size)
+{
+       const struct ipv6hdr *ip6h = ipv6_hdr(skb);
+
+       return __ipv6_tun_set_dst(&ip6h->saddr, &ip6h->daddr,
+                               ipv6_get_dsfield(ip6h), ip6h->hop_limit,
+                               ip6_flowlabel(ip6h), flags, tunnel_id,
+                               md_size);

   The continuation lines should start exactly under the 1st & on the broken
up line.
   That's DaveM's preference, I don't remember if checkpatch.pl reports that
for the networking code...

checkpatch doesn't report :(
I'll fix it to the next version.

[...]

MBR, Sergei