On 5/2/17 12:16 PM, Naiming Shen (naiming) via RT wrote:
> Hi Thomas,
>
> Thanks and I agree. What is the approacimate time of 4.4.0 of DHCP?
>
> Best Regards,
> - Naiming
>
>> On May 2, 2017, at 8:52 AM, Thomas Markwalder via RT <dhcp-suggest@isc.org> wrote:
>>
>> Hi Naiming:
>>
>> Thank you for your prompt reply.  I can understand you wanting to wait.  Since we've passed the last call it should be safe enough to develop the patch and just use place-holder values until there are official values rom IANA. Hopefully, we'll have them well before we release time.
>>
>> In the meantime, if you run into any issues or questions just let me know.
>>
>> Cheers,
>>
>> Thomas
>>
>> If you have any questions or run into issues please let me know.  
>> On Tue May 02 15:43:00 2017, naiming@cisco.com wrote:
>>> Hi Thomas,
>>>
>>> I’ll modified the patch of the ‘relay port’ feature in these two weeks
>>> based on your comments. Sorry for the delay.
>>> The working group has passed the Last Call on this draft, and
>>> I was waiting to see if we could get IANA official assigned numbers.
>>> But I think I’ll modify the patch first.
>>>
>>> Best Regards,
>>> - Naiming
>>>
>>>> On May 2, 2017, at 7:31 AM, Thomas Markwalder via RT <dhcp-
>>>> suggest@isc.org> wrote:
>>>>
>>>> Hello Naiming:
>>>>
>>>> I am following up on the review we provided for you back in February.
>>>> We are looking at feature content for our next feature release of ISC
>>>> DHCP, 4.4.0.
>>>>
>>>> Can you tell the status of your efforts there?  Do you have follow up
>>>> questions or feedback?
>>>>
>>>> Regards,
>>>>
>>>> Thomas Markwalder
>>>> ISC Software Engineering
>>>>
>>>>
>>>>
>>>> On Mon Feb 27 21:59:12 2017, naiming@cisco.com wrote:
>>>>> Hi Thomas,
>>>>>
>>>>> Thanks for the detailed review. and We’ll look over this and get
>>>>> back to you. thanks.
>>>>>
>>>>> - Naiming
>>>>>
>>>>>> On Feb 27, 2017, at 6:09 AM, Thomas Markwalder via RT <dhcp-
>>>>>> suggest@isc.org> wrote:
>>>>>>
>>>>>> Hello Naiming:
>>>>>>
>>>>>> First, I apologize for the delay in getting our comments back to
>>>>>> you
>>>>>> regarding the patch. In order to preserve formatting I have
>>>>>> attached
>>>>>> our
>>>>>> review as a text file.
>>>>>>
>>>>>> In general, the patch mechanics are fine, but we would rather see
>>>>>> the
>>>>>> the relay port specification done per interface rather than as
>>>>>> global
>>>>>> parameter.  Details on this are included in the attachment.
>>>>>>
>>>>>> To set expectations, these changes will not be included in our
>>>>>> upcoming maintenance release, 4.3.6, due out in May '17. Rather
>>>>>> they
>>>>>> would go into our feature release, 4.4.0, which is due out later in
>>>>>> the year, date is TBD.
>>>>>>
>>>>>> Thank you for your efforts thus far.  If you have any comments or
>>>>>> questions please let us know.
>>>>>>
>>>>>> Sincerely,
>>>>>>
>>>>>> Thomas Markwalder
>>>>>> ISC Software Engineering
>>>>>>
>>>>>>
>>>>>>
>>>>>> We would like to the patch split as follows:
>>>>>>
>>>>>> 1. V4 Server support
>>>>>> 2. V4 Relay support
>>>>>> 3. V6 Server support
>>>>>> 4. V6 Relay
>>>>>> 5. 4o6 Support
>>>>>>
>>>>>> 2. The BPF filter changes seem fine. They allows an interface to
>>>>>> accept
>>>>>> messages either from the standard port or the alternate port  which
>>>>>> matches
>>>>>> the RFC draft. For interfaces that are both up and down, though it
>>>>>> would
>>>>>> mean that the upstream could only receive on the alternate port as
>>>>>> the first
>>>>>> port in the filter would be the standard downstream port. This is
>>>>>> likely
>>>>>> acceptable versus more complicated filter logic.
>>>>>>
>>>>>> 3. The patch implements a global command line parameter, "-rp",
>>>>>> used
>>>>>> by
>>>>>> dhcrelay to specify an alternate port value. When this parameter is
>>>>>> specified
>>>>>> the interface discovery code has been modified to only register one
>>>>>> upstream
>>>>>> and one downstream interface.
>>>>>>
>>>>>> It is unclear if this was done for simplicity or based upon a
>>>>>> perceived most
>>>>>> likely use-case. There is a more flexible approach, that supports
>>>>>> multiple
>>>>>> interfaces, and has the additional benefit of being more seamless:
>>>>>>
>>>>>> Add an optional port value to the v4's "-i,-iu,-U" and v6's "-u"
>>>>>> command arguments:
>>>>>>
>>>>>> -iu interface[/port]
>>>>>>
>>>>>> -u [address%]interface[/port]
>>>>>>
>>>>>> This makes the alternate port use specific to an upstream
>>>>>> interface.
>>>>>> If users want only a single upstream or downstream interfaces they
>>>>>> explicitly configure it that way, if they want more than one they
>>>>>> can
>>>>>> still do that.
>>>>>>
>>>>>> The steps to implement this are outlined below:
>>>>>>
>>>>>> Rather than a single global relay_port variable, add a member for
>>>>>> it
>>>>>> to struct interface_info:
>>>>>>
>>>>>> struct interface_info {
>>>>>>   :
>>>>>>   unit16_t relay_port;
>>>>>> }
>>>>>>
>>>>>>
>>>>>> relay/dhcrealy.c:request_v4_interface()
>>>>>>  Modify to look for the port value and populate it in the
>>>>>> structure, only
>>>>>>  allowed if the flag parameter includes INTERFACE_UPSTREAM.
>>>>>>
>>>>>> relay/dhcrealy.c:parse_upstream()
>>>>>>  Modify to look for the port value and populate it in the
>>>>>> structure.
>>>>>>
>>>>>> discover.c::discover_interfaces()
>>>>>>  Would not require the changes made by the patch, it would revert
>>>>>> to as is.
>>>>>>
>>>>>>
>>>>>> Several places would need look at the structure port value, instead
>>>>>> of the global value:
>>>>>>
>>>>>> common/discover.c::lpf_gen_filter_setup():
>>>>>>
>>>>>> if ((udp.uh_dport != local_port) &&
>>>>>>   ((interface->relay_port == 0) || (udp.uh_dport != interface-
>>>>>>> relay_port)))
>>>>>>
>>>>>> common/packet.c:assemble_udp_ip_header():
>>>>>>
>>>>>> udp.uh_sport = (interface->relay_port ? interface->relay_port :
>>>>>> local_port);
>>>>>>
>>>>>> common/packet.c:decode_udp_ip_header():
>>>>>>
>>>>>> if ((udp.uh_dport != local_port) &&
>>>>>>   ((interface->relay_port == 0) || (udp.uh_dport != interface-
>>>>>>> relay_port)))
>>>>>> common/raw.c:if_register_send():
>>>>>>
>>>>>> name.sin_port = (info->relay_port ? info->relay_port: local_port;
>>>>>>
>>>>>> common/socket.c:if_register_socket():
>>>>>>
>>>>>> addr6->sin6_port = (info->relay_port ? info->relay_port:
>>>>>> local_port;
>>>>>>
>>>>>> The rest of the changes the patch makes to this function are
>>>>>> unnecessary.
>>>>>>
>>>>>> relay/dhcrelay.c:process_up6()
>>>>>>  Relocate the logic to add the D6O_RELAY_PORT to inside the
>>>>>> upstream send
>>>>>>  loop. Here the logic would need to be a little smarter:
>>>>>>
>>>>>> /* Send it to all upstreams. */
>>>>>> int has_relay_option = 0;
>>>>>> for (up = upstreams; up; up = up->next) {
>>>>>> if (has_relay_option) {
>>>>>>   /* Previous upstream added it, this either shouldn't
>>>>>>    * have it or needs to replace it, so let's remove it */
>>>>>>   delete_option(&dhcpv6_universe, opts, D6O_RELAY_PORT);
>>>>>>   has_relay_option = 0;
>>>>>> }
>>>>>>
>>>>>> if (upstream->relay_port || relay_client_port) {
>>>>>>   if (!save_option_buffer(&dhcpv6_universe, opts, NULL,
>>>>>>       (unsigned char *) &relay_client_port, sizeof(u_int16_t),
>>>>>>        D6O_RELAY_PORT, 0)) {
>>>>>>       log_error("Can't save relay-port.");
>>>>>>       option_state_dereference(&opts, MDL);
>>>>>>       return;
>>>>>>   }
>>>>>>
>>>>>> has_relay_option = 1;
>>>>>> }
>>>>>>
>>>>>> send_packet6(up->ifp, (unsigned char *) forw_data,
>>>>>>            (size_t) cursor, &up->link);
>>>>>> }
>>>>>>
>>>>>> Note these changes may not have covered everything necessary, and
>>>>>> of
>>>>>> course
>>>>>> have not been been compile so there may syntax errors. However they
>>>>>> should
>>>>>> suffice to guide the actual code work.
>>>>>>
>>>>>>
>>>>>> 4. The 4o6 patch seems fine. It adds room in the IPC header to
>>>>>> accommodate the
>>>>>> alternate port the V6 server received the message from so the v4
>>>>>> server can
>>>>>> echo it back. This allows the V6 server to send the response back
>>>>>> on
>>>>>> the
>>>>>> alternate port.
>>>>>>
>>>>
>>>>
>>
>>
>
>
That's yet to be decided. Possibly Q4/17.


Thomas