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

T5559: Add static neighbor-proxy feature #2240

Merged
merged 1 commit into from
Nov 7, 2023
Merged

T5559: Add static neighbor-proxy feature #2240

merged 1 commit into from
Nov 7, 2023

Conversation

sever-sever
Copy link
Member

@sever-sever sever-sever commented Sep 10, 2023 •
edited
Loading

Change Summary

Ability to set neighbor proxy ARP/NDP for specific hosts

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes)
  • Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
  • Other (please describe):

Related Task(s)

Component(s) name

proxy-arp, proxy-ndp

Proposed changes

How to test

VyOS configuration:

set protocols static neighbor-proxy arp 192.0.2.1 interface 'eth1'
set protocols static neighbor-proxy arp 192.0.2.1 interface 'eth0'
set protocols static neighbor-proxy arp 192.0.2.2 interface 'eth0'
set protocols static neighbor-proxy arp 192.0.2.3 interface 'eth0'
set protocols static neighbor-proxy nd 2001:db8::1 interface 'eth1'

Check:

vyos@r4# sudo ip neighbor show proxy
192.0.2.2 dev eth0 proxy 
192.0.2.3 dev eth0 proxy 
192.0.2.1 dev eth0 proxy 
192.0.2.1 dev eth1 proxy 
2001:db8::1 dev eth1 proxy

Checklist:

  • I have read the CONTRIBUTING document
  • I have linked this PR to one or more Phabricator Task(s)
  • I have run the components SMOKETESTS if applicable
  • My commit headlines contain a valid Task id
  • My change requires a change to the documentation
  • I have updated the documentation accordingly

@vyosbot vyosbot requested review from a team, dmbaturin, sarthurdev, zdc, jestabro and c-po and removed request for a team September 10, 2023 22:31
@c-po
Copy link
Member

c-po commented Sep 14, 2023

What about:

  • set protocols static arp 192.0.2.1 interface eth0 as an extension to the current static ARP <-> MAC CLI nodes we have?
  • set protocols static ndp 2001:db8::1 interface eth1
    ?

@c-po
Copy link
Member

c-po commented Oct 31, 2023

What about to summarize all under:

set protocols static neighbor-proxy arp 192.0.2.1 interface eth0
set protocols static neighbor-proxy arp 192.0.2.1 interface eth1
set protocols static neighbor-proxy nd 2001:db8::1 interface eth1

Then we will have a common root node for the feature for both address families. Does this also work for VRFs?

@sever-sever
Copy link
Member Author

What about to summarize all under:

set protocols static neighbor-proxy arp 192.0.2.1 interface eth0
set protocols static neighbor-proxy arp 192.0.2.1 interface eth1
set protocols static neighbor-proxy nd 2001:db8::1 interface eth1

Then we will have a common root node for the feature for both address families. Does this also work for VRFs?

@c-po Done!

set protocols static neighbor-proxy arp 192.0.2.1 interface 'eth1'
set protocols static neighbor-proxy arp 192.0.2.1 interface 'eth0'
set protocols static neighbor-proxy arp 192.0.2.2 interface 'eth0'
set protocols static neighbor-proxy arp 192.0.2.3 interface 'eth0'
set protocols static neighbor-proxy nd 2001:db8::1 interface 'eth1'

check

vyos@r4#  sudo ip neighbor show proxy
192.0.2.2 dev eth0 proxy 
192.0.2.3 dev eth0 proxy 
192.0.2.1 dev eth0 proxy 
192.0.2.1 dev eth1 proxy 
2001:db8::1 dev eth1 proxy

Not sure about VRF, it was a request from the task

@sever-sever sever-sever changed the title T5559: Add selective proxy ARP and NDP feature T5559: Add static neighbor-proxy feature Oct 31, 2023
@sever-sever sever-sever force-pushed the T5559 branch 2 times, most recently from 89e9a7c to 32f073b Compare October 31, 2023 10:21
c-po
c-po approved these changes Nov 1, 2023
View reviewed changes
interface-definitions/protocols_static_neighbor-proxy.xml.in Outdated
</constraint>
</properties>
<children>
<leafNode name="interface">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could use an include for the interface here

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

src/conf_mode/protocols_static_neighbor-proxy.py Outdated
if 'arp' in config:
for neighbor, neighbor_conf in config['arp'].items():
if 'interface' not in neighbor_conf:
raise ConfigError(f'neighbor-proxy arp {neighbor} interface required but not set.')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please reword the error message: „ARP neighbor-proxy for „IP“ requires an interface to be set!“
For both IPv4 and IPv6

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason that I use it

neighbor-proxy arp {neighbor} interface

It is the path of configuration that is not valid. It is clear where in the configuration you should add interface.

But sure, I can

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

@sever-sever
T5559: Add static neighbor-proxy feature
c56af99 
Ability to set ip neigbhor proxy

set protocols static neighbor-proxy arp 192.0.2.1 interface 'eth0'
set protocols static neighbor-proxy arp 192.0.2.2 interface 'eth0'
set protocols static neighbor-proxy nd 2001:db8::1 interface 'eth1'
@c-po c-po merged commit 4e8b16e into vyos:current Nov 7, 2023
@sever-sever
Copy link
Member Author

@Mergifyio backport sagitta

@mergify Mergify
Copy link
Contributor

mergify bot commented Nov 7, 2023 •
edited
Loading

backport sagitta

✅ Backports have been created

@mergify mergify bot mentioned this pull request Nov 7, 2023
Merged
c-po added a commit that referenced this pull request Nov 7, 2023
@c-po
Merge pull request #2458 from vyos/mergify/bp/sagitta/pr-2240
3238cb6 
T5559: Add static neighbor-proxy feature (backport #2240)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@c-po c-po c-po approved these changes

@dmbaturin dmbaturin Awaiting requested review from dmbaturin dmbaturin was automatically assigned from vyos/reviewers

@sarthurdev sarthurdev Awaiting requested review from sarthurdev sarthurdev was automatically assigned from vyos/reviewers

@zdc zdc Awaiting requested review from zdc zdc was automatically assigned from vyos/reviewers

@jestabro jestabro Awaiting requested review from jestabro jestabro was automatically assigned from vyos/reviewers

Assignees

@sever-sever sever-sever

Labels
current
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sever-sever @c-po