Add iptables4nattemplate chain element, refactor routelocalnet chain element #467

rejmond · 2022-05-10T14:41:39Z

Signed-off-by: Sergey Shlyanin sergey.shlyanin@xored.com

Description

Add iptables4nattemplate chain client element that apply iptables nat rules on Request and restore them on Close. iptables4nattemplate is as a part of connectioncontextkernel client chain
Turn routelocalnet to a client chain element and move it to connectioncontextkernel client chain
Add setroutelocalnet chain server element for setting a flag to kernel mechanism

Issue

#463
#464

denis-tingaikin

I think you could be instrested in this

sdk-kernel/pkg/kernel/tools/nshandle/ns_handle.go

Lines 37 to 53 in 0125eaf

    
           func FromURL(urlString string) (handle netns.NsHandle, err error) { 
        
           	var netNSURL *url.URL 
        
           	netNSURL, err = url.Parse(urlString) 
        
           	if err != nil || netNSURL.Scheme != "file" { 
        
           		return -1, errors.Wrapf(err, "invalid url: %v", urlString) 
        
           	} 
        
           	handle, err = netns.GetFromPath(netNSURL.Path) 
        
           	if err != nil { 
        
           		return -1, errors.Wrapf(err, "failed to obtain network NS handle") 
        
           	} 
        
           	return handle, nil 
        
           } 
        
           // RunIn runs runner in the given net NS 
        
           func RunIn(current, target netns.NsHandle, runner func() error) error {

denis-tingaikin · 2022-05-10T15:27:45Z

pkg/kernel/networkservice/applyiptables4nattemplate/server.go

+
+// NewServer - returns a new networkservice.NetworkServiceServer that modify IPTables rules
+// by mechanism provided template on Request and rollbacks rules changes on Close
+func NewServer(options ...Option) networkservice.NetworkServiceServer {


Do we really need options if dont have tests for this moment?

I left it for the future. I agree, it is better to remove unused feature it for now. Done.

denis-tingaikin · 2022-05-10T15:33:07Z

@edwarnicke , @rejmond Could we use this https://github.com/coreos/go-iptables/blob/master/iptables/iptables.go?

edwarnicke · 2022-05-10T15:34:38Z

@denis-tingaikin Does it simply anything? In the background I think its still just calling iptables, right?

denis-tingaikin · 2022-05-10T15:39:03Z

@edwarnicke yeah, you're right. I looked quickly and didnt check that. Exechelper is looking better to me here.

edwarnicke · 2022-05-10T15:43:46Z

@denis-tingaikin I had exactly the same thought you did and looked at this module over the weekend :) . But sometimes you notice details I miss, thus my question :)

denis-tingaikin

@rejmond Did we test this in real world?

denis-tingaikin · 2022-05-10T23:15:05Z

pkg/kernel/networkservice/applyiptables4nattemplate/common.go

+//go:build linux
+// +build linux
+
+package applyiptables4nattemplate


Suggested change

package applyiptables4nattemplate

package applyiptables

or

Suggested change

package applyiptables4nattemplate

package iptables

I think it could be better.

denis-tingaikin · 2022-05-12T11:34:38Z

pkg/kernel/networkservice/connectioncontextkernel/iptables4nattemplate/common.go

+}
+
+func (m *iptableManagerImpl) Apply(rulesTemplate string) error {
+	rules := strings.Split(rulesTemplate, ";")


Suggested change

rules := strings.Split(rulesTemplate, ";")

rules := strings.Split(rulesTemplate, "\n")

Apply accepts rules []string now, spliting is no longer required.

denis-tingaikin · 2022-05-12T11:35:02Z

pkg/kernel/networkservice/connectioncontextkernel/iptables4nattemplate/common.go

+
+	for _, rule := range rules {
+		// Remove possible tabs and spaces from a multistring template
+		rule = strings.Trim(rule, "\t ")


Suggested change

rule = strings.Trim(rule, "\t ")

rule = strings.TrimSpace(rule)

Removed Trim function at all.

denis-tingaikin

Added few comments others things are fine for the first step

denis-tingaikin · 2022-05-12T16:43:24Z

pkg/kernel/networkservice/connectioncontextkernel/iptables4nattemplate/client.go

+}
+
+func (c *iptablesClient) Request(ctx context.Context, request *networkservice.NetworkServiceRequest, opts ...grpc.CallOption) (*networkservice.Connection, error) {
+	ctxMap := metadata.Map(ctx, metadata.IsClient(c))


We also should consider about closing if someting went wrong

See at example

https://github.com/networkservicemesh/sdk-kernel/blob/main/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/ipaddress/client.go#L69 and https://github.com/networkservicemesh/sdk-kernel/blob/main/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/ipaddress/client.go#L77-L84

denis-tingaikin · 2022-05-12T17:46:58Z

@rejmond Could you please resolve conflicts? Please just run go get -u github.com/networkservicemesh/sdk@main and commit changes. Thanks.

…ernel chain * Turn routelocalnet chain element to a client element and move to cmnnectioncontextkernel chain * Add setroutelocalnet chain server element Signed-off-by: Sergey Shlyanin <sergey.shlyanin@xored.com>

… into issue-463

…k-kernel@main PR link: networkservicemesh/sdk-kernel#467 Commit: e0f2b84 Author: Denis Tingaikin Date: 2022-05-13 12:42:28 +0300 Message: - Merge pull request #467 from rejmond/issue-463 Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>

rejmond force-pushed the issue-463 branch from 159d8f0 to 19b982a Compare May 10, 2022 14:43

denis-tingaikin reviewed May 10, 2022

View reviewed changes

rejmond force-pushed the issue-463 branch 2 times, most recently from a1aedda to 21c1649 Compare May 10, 2022 14:58

rejmond marked this pull request as draft May 10, 2022 15:13

rejmond marked this pull request as ready for review May 10, 2022 15:14

denis-tingaikin reviewed May 10, 2022

View reviewed changes

rejmond force-pushed the issue-463 branch 8 times, most recently from eb7f421 to 279e41b Compare May 10, 2022 17:20

rejmond requested a review from edwarnicke May 10, 2022 17:45

denis-tingaikin reviewed May 10, 2022

View reviewed changes

rejmond removed the request for review from edwarnicke May 11, 2022 13:44

rejmond force-pushed the issue-463 branch 3 times, most recently from 2b53a6d to 42c13d3 Compare May 11, 2022 15:46

rejmond changed the title ~~Add applyiptables4nattemplate chain element~~ Add iptables4nattemplate chain element, refactor routelocalnet chain element May 11, 2022

denis-tingaikin approved these changes May 12, 2022

View reviewed changes

denis-tingaikin reviewed May 12, 2022

View reviewed changes

rejmond force-pushed the issue-463 branch 2 times, most recently from 9bd5e37 to 0023de2 Compare May 12, 2022 17:39

denis-tingaikin approved these changes May 12, 2022

View reviewed changes

rejmond force-pushed the issue-463 branch 3 times, most recently from 15f1b77 to bac4bfb Compare May 12, 2022 17:58

rejmond force-pushed the issue-463 branch from bac4bfb to 5acbf94 Compare May 12, 2022 18:01

Merge branch 'main' of ssh://github.com/networkservicemesh/sdk-kernel…

1125d6c

… into issue-463

denis-tingaikin merged commit e0f2b84 into networkservicemesh:main May 13, 2022

This was referenced May 13, 2022

Update from update/networkservicemesh/sdk-kernel networkservicemesh/sdk-sriov#412

Merged

Update from update/networkservicemesh/sdk-kernel networkservicemesh/sdk-vpp#583

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add iptables4nattemplate chain element, refactor routelocalnet chain element #467

Add iptables4nattemplate chain element, refactor routelocalnet chain element #467

rejmond commented May 10, 2022 •

edited

Loading

denis-tingaikin left a comment •

edited

Loading

denis-tingaikin May 10, 2022

rejmond May 10, 2022

denis-tingaikin commented May 10, 2022

edwarnicke commented May 10, 2022

denis-tingaikin commented May 10, 2022 •

edited

Loading

edwarnicke commented May 10, 2022

denis-tingaikin left a comment

denis-tingaikin May 10, 2022 •

edited

Loading

denis-tingaikin May 12, 2022

rejmond May 12, 2022

denis-tingaikin May 12, 2022

rejmond May 12, 2022

denis-tingaikin left a comment

denis-tingaikin May 12, 2022 •

edited

Loading

denis-tingaikin commented May 12, 2022 •

edited

Loading

	func FromURL(urlString string) (handle netns.NsHandle, err error) {
	var netNSURL *url.URL
	netNSURL, err = url.Parse(urlString)
	if err != nil \|\| netNSURL.Scheme != "file" {
	return -1, errors.Wrapf(err, "invalid url: %v", urlString)
	}

	handle, err = netns.GetFromPath(netNSURL.Path)
	if err != nil {
	return -1, errors.Wrapf(err, "failed to obtain network NS handle")
	}

	return handle, nil
	}

	// RunIn runs runner in the given net NS
	func RunIn(current, target netns.NsHandle, runner func() error) error {

	rules := strings.Split(rulesTemplate, ";")
	rules := strings.Split(rulesTemplate, "\n")

	rule = strings.Trim(rule, "\t ")
	rule = strings.TrimSpace(rule)

Add iptables4nattemplate chain element, refactor routelocalnet chain element #467

Add iptables4nattemplate chain element, refactor routelocalnet chain element #467

Conversation

rejmond commented May 10, 2022 • edited Loading

Description

Issue

denis-tingaikin left a comment • edited Loading

Choose a reason for hiding this comment

denis-tingaikin May 10, 2022

Choose a reason for hiding this comment

rejmond May 10, 2022

Choose a reason for hiding this comment

denis-tingaikin commented May 10, 2022

edwarnicke commented May 10, 2022

denis-tingaikin commented May 10, 2022 • edited Loading

edwarnicke commented May 10, 2022

denis-tingaikin left a comment

Choose a reason for hiding this comment

denis-tingaikin May 10, 2022 • edited Loading

Choose a reason for hiding this comment

denis-tingaikin May 12, 2022

Choose a reason for hiding this comment

rejmond May 12, 2022

Choose a reason for hiding this comment

denis-tingaikin May 12, 2022

Choose a reason for hiding this comment

rejmond May 12, 2022

Choose a reason for hiding this comment

denis-tingaikin left a comment

Choose a reason for hiding this comment

denis-tingaikin May 12, 2022 • edited Loading

Choose a reason for hiding this comment

denis-tingaikin commented May 12, 2022 • edited Loading

rejmond commented May 10, 2022 •

edited

Loading

denis-tingaikin left a comment •

edited

Loading

denis-tingaikin commented May 10, 2022 •

edited

Loading

denis-tingaikin May 10, 2022 •

edited

Loading

denis-tingaikin May 12, 2022 •

edited

Loading

denis-tingaikin commented May 12, 2022 •

edited

Loading