Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Per Vlan DHCP relay is broken if DHCP server is connected via Vlan interface #1247

Closed
johcheun opened this issue Dec 18, 2017 · 3 comments · Fixed by #2946
Closed

Per Vlan DHCP relay is broken if DHCP server is connected via Vlan interface #1247

johcheun opened this issue Dec 18, 2017 · 3 comments · Fixed by #2946
Assignees
@jleveque
Labels
Question ❓

Comments

@johcheun
Copy link
Contributor

Description
Prior to the per vlan dhcp relay support was added via 1d16a37 , DhcpResources tag was used to specify the Dhcp relay server information, and all routed interfaces, VLAN interfaces and Port-Channel interfaces were added to listening interfaces for the dhcrelay command in the j2 template.

After the commit with the new DhcpRelay tag, only the dhcp relay vlan, routed interfaces and Port-Channel interfaces are added to the dhcrelay command, but if the dhcp server sits behind a different vlan, the dhcp offer is not getting forwarded.

Steps to reproduce the issue:

  1. Configure per vlan dhcp relay
  2. Have a dhcp server connected to a VLAN interface in the topology
  3. Have dhcp relay configured on another VLAN interface.

Describe the results you received:
DHCP request is getting forwarded to the dhcp server, but DHCP offer received from the server is dropped since it is not in the dhcrelay listening interfaces.

Describe the results you expected:
DHCP offer should be forwarded properly in the case of dhcp server connected to vlan interface.

Additional information you deem important (e.g. issue happens only occasionally):
Manually modified docker-dhcp-relay.supervisord.conf to include the dhcp server VLAN, and dhcp relay is working properly.
@jleveque jleveque self-assigned this Dec 18, 2017
@jleveque
Copy link
Contributor

Hi @johcheun. We have moved to a DHCP-relay-per-VLAN model in order to support specifying a different set of DHCP servers for servers under each VLAN. This allows for DHCP traffic to only travel between the appropriate clients and servers. This configuration assumes all DHCP servers are reachable via uplink interfaces. We have not encountered the scenario where a DHCP server(s) resides under antoher VLAN of the same switch.

The best solution I can think of to support both scenarios is to be able to specify on which interfaces the relay should listen for requests from clients (downstream interfaces) and which interfaces to listen for replies from DHCP servers (upstream interfaces). With this functionality, the one VLAN containing DHCP clients can be specified as a downstream interface and all other interfaces, including other VLANs can be specified as upstream interfaces.

Unfortunately, the version of isc-dhcp-relay that we are currently using (v4.3.3-6) does not support specifying whether an IPv4 interface is an upstream or downstream interface. All interfaces specified using the -i parameter are listened on for both requests and replies. But I just looked into upstream source of isc-dhcp-relay, and it appears they have since added the ability to specify upstream (-iu) and downstream (-id) v4 interfaces. Unfortunately, all versions > v4.3.3-6 require dependency packages that are not available in Debian Jessie, so we currently cannot compile or install a newer version.

That said, we are targeting upgrading the underlying SONiC distro from Debian Jessie to Debian Stretch for our 03/15/18 release, at which time we will be able to upgrade to the latest version of isc-dhcp-relay which will offer this support. Hopefully this timeline is reasonable for you, since you seem to have a simple workaround in the meantime.

@johcheun
Copy link
Contributor Author

@jleveque Sounds good. Actually with the flexibility of specifying upstream / downstream dhcp port, it could further eliminate unwanted dhcp traffic to other segments of the network. Thanks.

@klhaung klhaung mentioned this issue Sep 3, 2018
Closed
@jleveque jleveque mentioned this issue May 3, 2019
Merged
@jleveque jleveque mentioned this issue May 18, 2019
Merged
@jleveque jleveque mentioned this issue May 26, 2019
Merged
@jleveque
Copy link
Contributor

jleveque commented Jun 3, 2019

Closed as this is resolved by the following commits on the respective branches:

@dzhangalibaba dzhangalibaba mentioned this issue Apr 20, 2020
Merged
@judyjoseph judyjoseph mentioned this issue Apr 25, 2020
Merged
@jleveque jleveque mentioned this issue Nov 18, 2020
Merged
jleveque added a commit that referenced this issue Nov 25, 2020
@jleveque
[sonic-utilities] Update submodule; Build and install as a Python 3 w…
7f4ab8f 
…heel (#5926)

Submodule updates include the following commits:

* src/sonic-utilities 9dc58ea...f9eb739 (18):
  > Remove unnecessary calls to str.encode() now that the package is Python 3; Fix deprecation warning (#1260)
  > [generate_dump] Ignoring file/directory not found Errors (#1201)
  > Fixed porstat rate and util issues (#1140)
  > fix error: interface counters is mismatch after warm-reboot (#1099)
  > Remove unnecessary calls to str.decode() now that the package is Python 3 (#1255)
  > [acl-loader] Make list sorting compliant with Python 3 (#1257)
  > Replace hard-coded fast-reboot with variable. And some typo corrections (#1254)
  > [configlet][portconfig] Remove calls to dict.has_key() which is not available in Python 3 (#1247)
  > Remove unnecessary conversions to list() and calls to dict.keys() (#1243)
  > Clean up LGTM alerts (#1239)
  > Add 'requests' as install dependency in setup.py (#1240)
  > Convert to Python 3 (#1128)
  > Fix mock SonicV2Connector in python3: use decode_responses mode so caller code will be the same as python2 (#1238)
  > [tests] Do not trim from PATH if we did not append to it; Clean up/fix shebangs in scripts (#1233)
  > Updates to bgp config and show commands with BGP_INTERNAL_NEIGHBOR table (#1224)
  > [cli]: NAT show commands newline issue after migrated to Python3 (#1204)
  > [doc]: Update Command-Reference.md (#1231)
  > Added 'import sys' in feature.py file (#1232)

* src/sonic-py-swsssdk 9d9f0c6...1664be9 (2):
  > Fix: no need to decode() after redis client scan, so it will work for both python2 and python3 (#96)
  > FieldValueMap `contains`(`in`)  will also work when migrated to libswsscommon(C++ with SWIG wrapper) (#94)

- Also fix Python 3-related issues:
    - Use integer (floor) division in config_samples.py (sonic-config-engine)
    - Replace print statement with print function in eeprom.py plugin for x86_64-kvm_x86_64-r0 platform
    - Update all platform plugins to be compatible with both Python 2 and Python 3
    - Remove shebangs from plugins files which are not intended to be executable
    - Replace tabs with spaces in Python plugin files and fix alignment, because Python 3 is more strict
    - Remove trailing whitespace from plugins files
santhosh-kt pushed a commit to santhosh-kt/sonic-buildimage that referenced this issue Feb 25, 2021
@jleveque @santhosh-kt
[sonic-utilities] Update submodule; Build and install as a Python 3 w…
8d48e6f 
…heel (sonic-net#5926)

Submodule updates include the following commits:

* src/sonic-utilities 9dc58ea...f9eb739 (18):
  > Remove unnecessary calls to str.encode() now that the package is Python 3; Fix deprecation warning (sonic-net#1260)
  > [generate_dump] Ignoring file/directory not found Errors (sonic-net#1201)
  > Fixed porstat rate and util issues (sonic-net#1140)
  > fix error: interface counters is mismatch after warm-reboot (sonic-net#1099)
  > Remove unnecessary calls to str.decode() now that the package is Python 3 (sonic-net#1255)
  > [acl-loader] Make list sorting compliant with Python 3 (sonic-net#1257)
  > Replace hard-coded fast-reboot with variable. And some typo corrections (sonic-net#1254)
  > [configlet][portconfig] Remove calls to dict.has_key() which is not available in Python 3 (sonic-net#1247)
  > Remove unnecessary conversions to list() and calls to dict.keys() (sonic-net#1243)
  > Clean up LGTM alerts (sonic-net#1239)
  > Add 'requests' as install dependency in setup.py (sonic-net#1240)
  > Convert to Python 3 (sonic-net#1128)
  > Fix mock SonicV2Connector in python3: use decode_responses mode so caller code will be the same as python2 (sonic-net#1238)
  > [tests] Do not trim from PATH if we did not append to it; Clean up/fix shebangs in scripts (sonic-net#1233)
  > Updates to bgp config and show commands with BGP_INTERNAL_NEIGHBOR table (sonic-net#1224)
  > [cli]: NAT show commands newline issue after migrated to Python3 (sonic-net#1204)
  > [doc]: Update Command-Reference.md (sonic-net#1231)
  > Added 'import sys' in feature.py file (sonic-net#1232)

* src/sonic-py-swsssdk 9d9f0c6...1664be9 (2):
  > Fix: no need to decode() after redis client scan, so it will work for both python2 and python3 (sonic-net#96)
  > FieldValueMap `contains`(`in`)  will also work when migrated to libswsscommon(C++ with SWIG wrapper) (sonic-net#94)

- Also fix Python 3-related issues:
    - Use integer (floor) division in config_samples.py (sonic-config-engine)
    - Replace print statement with print function in eeprom.py plugin for x86_64-kvm_x86_64-r0 platform
    - Update all platform plugins to be compatible with both Python 2 and Python 3
    - Remove shebangs from plugins files which are not intended to be executable
    - Replace tabs with spaces in Python plugin files and fix alignment, because Python 3 is more strict
    - Remove trailing whitespace from plugins files
stepanblyschak pushed a commit to stepanblyschak/sonic-buildimage that referenced this issue May 10, 2021
@jleveque
[configlet][portconfig] Remove calls to dict.has_key() which is not a…
9d55082 
…vailable in Python 3 (sonic-net#1247)

Replace calls to `dict.has_key()` with `in`. It appears the `2to3` tool missed these.
abdosi added a commit that referenced this issue Oct 8, 2021
@abdosi
[Submodule update] sonic-swss
3cb16cc 
901e2efb4f5c6a1179068b43e6a837dfff19d9d9 (HEAD -> 201911, origin/201911) [fpmsyncd] Skip routes to eth0 or docker0 (#1606)
6c6f4fbbeb24b6e16a2bffdee760ba6a95e2ceb3 configure extra inc/lib directory for build (#1247)
cccb59efa031b817c1936a74db969debbb9b2775 [201911][Cherry-pick] [acl mirror action] Mirror session ref count fix at acl rule attachment (#1896)
62dc36d9da988807c7a397fd90ac2859a5a07699 [ci]: Support Azure pipelines on 201911 branch (#1806)

Signed-off-by: Abhishek Dosi <abdosi@microsoft.com>
theasianpianist pushed a commit to theasianpianist/sonic-buildimage that referenced this issue Feb 5, 2022
@lguohan
configure extra inc/lib directory for build (sonic-net#1247)
eea6815 
Signed-off-by: Guohan Lu <lguohan@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jleveque jleveque

Labels
Question ❓
Projects
None yet
Milestone
No milestone
2 participants
@jleveque @johcheun