In article <75925357-8e16-0f0f-b7a0-78155c865...@iij.ad.jp>,
Kengo NAKAHARA <k-nakah...@iij.ad.jp> wrote:
>Hi,
>
>On 2017/12/19 2:54, Christos Zoulas wrote:
>> In article <02c36311-2fcd-08cf-cc71-b472e7c01...@iij.ad.jp>,
>> Kengo NAKAHARA <k-nakah...@iij.ad.jp> wrote:
>>> Hi,
>>>
>>> We implement ipsec(4) pseudo interface for route-based VPNs. This pseudo
>>> interface manages its security policy(SP) by itself, in particular, we do
>>> # ifconfig ipsec0 tunnel 10.0.0.1 10.0.0.2
>>> the SPs "10.0.0.1 -> 10.0.0.2"(out) and "10.0.0.2 -> 10.0.0.1"(in) are
>>> generated automatically and atomically. And then, when we do
>>> # ifconfig ipsec0 deletetunnel
>>> the SPs are destroyed automatically and atomically, too.
>>>
>>> Here is the patches and an unified patch.
>>> http://netbsd.org/~knakahara/if_ipsec/if_ipsec.tgz
>>> http://netbsd.org/~knakahara/if_ipsec/if_ipsec-unified.patch
>>>
>>> By the way, I have one question. In the above patch(s), I temporarily add
>>> manual for ipsecX pseudo interface as if_ipsec.4, because there is already
>>> ipsec.4 for general ipsec protocol. How should I add the man of ipsec(4)
>>> pseudo interface?
>>> (a) Add if_ipsec.4
>>> (b) move current ipsec.4(for ipsec protocol) to ipsec.9, and then
>>> add ipsec.4(for ipsec pseudo interface)
>>> (c) any other
>>>
>>> Could you comment the patch or the question?
>>
>> I've wanted this feature for a long time! Looks ok to me, but the
>> sockaddr_copy()/port setting code, should be abstracted to a single
>> function since it is repeated in ioctl().
>
>Thank you for your reviewing. I fix it in the following patch.
> - patch series
> - https://netbsd.org/~knakahara/if_ipsec/if_ipsec2.tgz
> - unified patch
> - https://netbsd.org/~knakahara/if_ipsec/if_ipsec2-unified.patch
Thanks:
+ error = var->iv_output(var, family, m);
+ if (!error) {
- It is simpler to do (early returns):
if (error)
return error;
...
return 0;
- What's the point of 'goto bad' in ioctl if there is no cleanup to be done?
- There is one more place you could use the addr_port function?
[or perhaps abstract it in in_port_t *get_port(struct sockaddr *)?
- We should use in_port_t instead of unsigned short for ports.
- I am not clear how the code interracts with ipsec endpoints created
from /etc/ipsec.conf and ones created via ifconfig? I.e. if I create
endpoints in /etc/ipsec.conf, does the cloner interface get automatically
constructed?
Thanks,
christos
