On Wed, Feb 27, 2019 at 01:01:37PM +0000, Stuart Henderson wrote:
> On 2019/02/26 16:34, Tobias Heider wrote:
> > Hi,
> > 
> > this diff adds support for IKEv2 Message Fragmentation as defined in
> > RFC 7383 (https://tools.ietf.org/html/rfc7383) to iked(8).
> 
> Thank you, I know there are quite a few additions in the genua tree and
> splitting them out is not easy. I'm running this in a few places now,
> have tested both with and without fragmentation, no problems seen.

Actually, this is not even part of the genua tree.  He implemented it
here the last two weeks and tested it using bluhm@'s test machines. :)

Patrick

> A couple of nits - manpage part is missing, I propose this:
> 
> Index: iked.conf.5
> ===================================================================
> RCS file: /cvs/src/sbin/iked/iked.conf.5,v
> retrieving revision 1.53
> diff -u -p -r1.53 iked.conf.5
> --- iked.conf.5       31 Jan 2018 13:25:55 -0000      1.53
> +++ iked.conf.5       27 Feb 2019 12:45:46 -0000
> @@ -136,6 +136,12 @@ This is the default.
>  .It Ic set decouple
>  Don't load the negotiated SAs and flows from the kernel.
>  This mode is only useful for testing and debugging.
> +.It Ic set fragmentation
> +Enable IKEv2 Message Fragmentation (RFC 7383) support.
> +This allows IKEv2 to operate in environments that might block IP fragments.
> +.It Ic set nofragmentation
> +Disables IKEv2 Message Fragmentation support.
> +This is the default.
>  .It Ic set mobike
>  Enable MOBIKE (RFC 4555) support.
>  This is the default.
> 
> There are some over-long lines - it's not fair to require <=80 for
> everything in this diff when big parts of iked already exceed this,
> but some of the longer ones need reining in e.g.
> 
> > Index: sbin/iked/iked.h
> ..
>    
> |------------------------------------------------------------------------------|
> > +#define IKED_FRAG_TOTAL_MAX        111                     /* upper limit 
> > of frag_total (64kB / 576B) */
> ...#define IKED_FRAG_TOTAL_MAX        111     /* upper limit of frag_total 
> (64kB / 576B) */
> 
> >  struct iked_message *
> >      ikev2_msg_lookup(struct iked *, struct iked_msgqueue *,
> >         struct iked_message *, struct ike_header *);
> > +void        ikev2_msg_lookup_dispose_all(struct iked *env, struct 
> > iked_msgqueue *queue,
> > +       struct iked_message *msg, struct ike_header *hdr);
> > +int         ikev2_msg_lookup_retransmit_all(struct iked *env, struct 
> > iked_msgqueue *queue,
> > +       struct iked_message *msg, struct ike_header *hdr, struct iked_sa 
> > *sa);
> ...(split onto another line)
> 
> > Index: sbin/iked/ikev2.c
> ..
> > -           if ((m = ikev2_msg_lookup(env, &sa->sa_responses, msg, hdr))) {
> > -                   if (ikev2_msg_retransmit_response(env, sa, m)) {
> > +           if ((r = ikev2_msg_lookup_retransmit_all(env, 
> > &sa->sa_responses, msg, hdr, sa)) != 0) {
> > +                   if (r == -1) {
> ...(and here)
> 
> I've done that in my tree.
> 
> Are there comments from anyone else?
> 

Reply via email to