Subject: Re: [PATCH RFC 1/5] {pktgen, xfrm} Remove original pktgen ipsec fixed configuration

From: Sergei Shtylyov <[email-masked]>
Date: Thu, 05 Dec 2013 18:00:05 +0400

Hello.

On 05-12-2013 11:57, Fan Du wrote:

> Cleanup original fixed IPsec output mode and encapuslation type,
> As following patchset enable user to configure IPsec attribute.

> Signed-off-by: Fan Du <[email-masked]>
> ---
>   net/core/pktgen.c |   11 +++--------
>   1 file changed, 3 insertions(+), 8 deletions(-)

> diff --git a/net/core/pktgen.c b/net/core/pktgen.c
> index a797fff..ab67986 100644
> --- a/net/core/pktgen.c
> +++ b/net/core/pktgen.c
> @@ -387,8 +387,7 @@ struct pktgen_dev {
>   	int node;               /* Memory node */
>
>   #ifdef CONFIG_XFRM
> -	__u8	ipsmode;		/* IPSEC mode (config) */
> -	__u8	ipsproto;		/* IPSEC type (config) */
> +

    Why add this empty line which you'll remove in patch #3? And I'd also have 
removed empty #ifdef too...

>   #endif
>   	char result[512];
>   };
> @@ -2482,10 +2481,7 @@ static int pktgen_output_ipsec(struct sk_buff *skb, struct pktgen_dev *pkt_dev)
>
>   	if (!x)
>   		return 0;
> -	/* XXX: we dont support tunnel mode for now until
> -	 * we resolve the dst issue */
> -	if (x->props.mode != XFRM_MODE_TRANSPORT)
> -		return 0;
> +

    Why? There's enough empty lines already.

>
>   	spin_lock(&x->lock);
>
> @@ -3540,8 +3536,7 @@ static int pktgen_add_device(struct pktgen_thread *t, const char *ifname)
>   		goto out2;
>   	}
>   #ifdef CONFIG_XFRM
> -	pkt_dev->ipsmode = XFRM_MODE_TRANSPORT;
> -	pkt_dev->ipsproto = IPPROTO_ESP;
> +

    Same question about empty line removed in patch #3.

>   #endif

WBR, Sergei