Added changes to permit pppoe interfaces #38
Conversation
src/config.c
Outdated
| @@ -396,7 +396,7 @@ struct SubnetList *parseSubnetAddress(char *addrstr) { | |||
| } | |||
|
|
|||
| tmpSubnet = (struct SubnetList*) malloc(sizeof(struct SubnetList)); | |||
| tmpSubnet->subnet_addr = addr; | |||
| tmpSubnet->subnet_addr = (addr & mask); | |||
There was a problem hiding this comment.
Because in ifvc.c are now handling subnets on the form addr & mask so it must be changed in order to work properly.
There was a problem hiding this comment.
I just would like to note that this specific change was reverted on pfSense
There was a problem hiding this comment.
Ok, so this change must be reverted....
There was a problem hiding this comment.
You are right, yesterday night i had compiled the source code and tested it. It is working perfectly without this change, and i can confirm that igmpproxy is working with a vlan tagged with pppoe interface.
What are next steps?. Do I cancel this pull request and open another one without config.c change or Do I revert the change in this pull request?
There was a problem hiding this comment.
Please update this pull request. There is no need to create a new one.
| @@ -274,6 +281,21 @@ void buildIfVc(void) { | |||
| mask = s_addr_from_sockaddr(&IfReq.ifr_addr); // Do not use ifr_netmask as it is not available on freebsd | |||
| subnet = addr & mask; | |||
|
|
|||
| dp = getIfByName(IfPt->ifr_name, 1); | |||
There was a problem hiding this comment.
What is the purpose of function parameter which is always constant?
There was a problem hiding this comment.
It is to be able to take advantage of the function so that it also works in the same way as it did in the past.
There was a problem hiding this comment.
But that function is not called from other places. Such change introduce a dead code as parameter is never false.
| @@ -201,8 +201,10 @@ void rebuildIfVc () { | |||
| ** | |||
| */ | |||
| void buildIfVc(void) { | |||
| struct ifreq IfVc[ sizeof( IfDescVc ) / sizeof( IfDescVc[ 0 ] ) ]; | |||
| struct ifreq IfVc[ MAX_IF * 2 ]; /* allocate in double in order to get as much information as possible */ | |||
There was a problem hiding this comment.
Why *2? Maximal number is maximal. So either defined constant is not so large or there is another hidden problem?
| */ | ||
| if ( IfPt->ifr_addr.sa_family != AF_INET ) { | ||
| my_log(LOG_DEBUG, 0, "Interface is not AF_INET, skipping %s (family %d)", IfPt->ifr_name, IfPt->ifr_addr.sa_family); | ||
| continue; |
There was a problem hiding this comment.
Skipping interfaces without IPv4 address would mean igmpproxy itself would not see this interface. Why such change is needed? Previously these interfaces was normally processed just marked as non-IPv4.
| while (currsubnet->next != NULL) | ||
| currsubnet = currsubnet->next; | ||
| currsubnet->next = allowednet; | ||
| continue; |
There was a problem hiding this comment.
This completely skip code which is below. Why?
Looks like this is a hack which just hide the fact that there is a probably bug in code below and try to invisibly skip it. This change is non-intuitive and introduce (potentially) dead code. Also probably may introduce another bugs as parts like IfDescEp++; are not executed...
|
@ljpaff Could you please address issues above? |
Workaround for:
Igmpproxy is not starting if upstream interface is a pppoe interface #28